Skip to content

Part 1: Py2->3 Futurize stage 2 without str-unicode, urllib changes (#406)#411

Merged
inferno-chromium merged 7 commits into
masterfrom
b1
Apr 26, 2019
Merged

Part 1: Py2->3 Futurize stage 2 without str-unicode, urllib changes (#406)#411
inferno-chromium merged 7 commits into
masterfrom
b1

Conversation

@inferno-chromium

Copy link
Copy Markdown
Collaborator

No description provided.

@googlebot googlebot added the cla: yes CLA signed. label Apr 26, 2019
@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

This patch does

git ls-files "*.py" | xargs futurize -w -n --stage2 -x libfuturize.fixes.fix_unicode_keep_u -x libfuturize.fixes.fix_future_standard_library_urllib -x libfuturize.fixes.fix_future_standard_library -x libfuturize.fixes.fix_object -x libfuturize.fixes.fix_basestring -x libfuturize.fixes.fix_future_builtins

excludes some things that will take more time and will go in Q3 i think.

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

had to fix up a lot of old_div after this futurize step, but i think good for cleanup.

@oliverchang oliverchang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with just a minor remark. Looks like it's also failing lint right now.

Comment thread src/appengine/handlers/cron/grouper.py Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little surprised by these changes, since the previous version was using generators and this change makes it always convert the whole thing to a list. I would've expected the change to omit the list() call so that under Python 3 the behaviour is the same as before.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/questions/17695456/why-does-python-3-need-dict-items-to-be-wrapped-with-list :( :( i can remove all the list() since it was just overcautionary but we probably have to rerun futurize sometime and need to find way so that it does not reconvert it again.

@inferno-chromium inferno-chromium Apr 26, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably will convert to option 2 in https://python-future.org/compatible_idioms.html#iterating-through-dict-keys-values-items

# Python 2 and 3: option 3
from future.utils import iteritems
# or
from six import iteritems

for (key, value) in iteritems(heights):
    ...

Will think more tmrw.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using six sounds good! Converting everything to a list could potentially have issues because of extra memory etc.

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium inferno-chromium changed the title Part 1: Py2->3 Futurize stage 2 without str-unicode, urllib changes. Part 1: Py2->3 Futurize stage 2 without str-unicode, urllib changes (#406) Apr 26, 2019
@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@inferno-chromium

Copy link
Copy Markdown
Collaborator Author

/gcbrun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes CLA signed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants