-
-
Notifications
You must be signed in to change notification settings - Fork 628
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
Python 3: dictionary iteration/list methods made compatible with Python 3 #9671
Merged
michaelDCurran
merged 24 commits into
nvaccess:threshold_py3_staging
from
josephsl:py3dictIterMethods
Jun 8, 2019
Merged
Python 3: dictionary iteration/list methods made compatible with Python 3 #9671
michaelDCurran
merged 24 commits into
nvaccess:threshold_py3_staging
from
josephsl:py3dictIterMethods
Jun 8, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…all. Re nvaccess#9067. Modules such as UIA handler, settings dialogs and others use dict.items/keys/values. This means in Python 2, it returns a list, whereas it returns an iterator in Python 3. Therefore wrap these inside a list call to preserve semantics (also include notes for some of these).
…t.iterkeys -> dict.keys. Re nvaccess#9067.
…alues -> dict.values. Re nvaccess#9067.
…itervalues -> dict.values. Re nvaccess#9067.
…rvalues -> dict.values. Re nvaccess#9067.
… -> dict.values. Re nvaccess#9067.
josephsl
changed the title
Python 3: dictionary iteration/list methods made compatbiel with Python 3
Python 3: dictionary iteration/list methods made compatible with Python 3
Jun 5, 2019
Hi, Commit count: this is because it is divided into two major themes: saving existing dict.items/keys/values, and then modifying iter* methods based on modules. Thanks. |
Have you also touched the config module iterkeys methods? I think it makes sense to keep these in sync with the Python version of use.
|
Collaborator
Author
Hi, no, as I felt it is too risky to edit them for now. Once we determine it is safe to do so, then I think we should do something about them – perhaps after the transition or as part of that. Thanks.
|
Makes sense. However, just in case you haven't already done that, could you add some comments o that code that explains why this case of iteritems isn't standard and should stay for now, also with the request to revisit this at some point in the near future?
|
Collaborator
Author
Hi, I’ll do that in a few hours. Thanks.
|
michaelDCurran
approved these changes
Jun 8, 2019
feerrenrut
reviewed
Jun 8, 2019
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.
Looks good to me, thanks @josephsl
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #9067
Summary of the issue:
Python 2 and 3 have different expectations ofr dictionary iteration, including dict.iteritems/iterkeys/itervalues vs dict.items/keys/values and related functionality.
Description of how this pull request fixes the issue:
The following changes were made:
Testing performed:
Tested with Python 3 (the interpreter and NVDA source code), along with testing equivalent functionality in Python 2 (interpreter and Python Console).
Known issues with pull request:
There might be some more dictionary iteration methods that might be present.
Change log entry:
None
Additional notes:
For dict.iterkeys -> dict.keys, Python 3 may ask us to just use "for something in dict". This should be deferred until Python 3 transition is complete.
Thanks.