-
-
Notifications
You must be signed in to change notification settings - Fork 423
Remove python 2 __future__ syntax #1864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1864 +/- ##
==========================================
- Coverage 64.44% 64.37% -0.08%
==========================================
Files 200 200
Lines 16033 15998 -35
==========================================
- Hits 10333 10298 -35
Misses 5700 5700
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Just need @bsipocz or @keflavich to take a last look at it and it should be ready to merge.
Thank you @daisycodes! I totally agree that changing the prints the 2to3 way makes little sense, so thanks for not pushing those to the PR. |
This PR removes the use of
from __future__
usages in the codebase. I'm using the2to3
translator which comes with the python interpreter as a script. By default it uses a lot of fixers which made changes I wasn't sure is what we want. For example this linewas replaced with
And also calls to the
items()
method of dictionaries were wrapped arounded thelist()
function. While all these are valid code, I just don't see the point and felt like these will just increase code review time. What are your thoughts on these modifications and is there another approach to removing all python2 code in a sensible way?I'm not adding the closes section of this PR because I don't want the issue to be closed when this PR gets merged as it doesn't resolve the ticket completely.