Skip to content
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: work with different dictionary walking methods #9067

Closed
josephsl opened this issue Dec 14, 2018 · 7 comments
Closed

Python 3: work with different dictionary walking methods #9067

josephsl opened this issue Dec 14, 2018 · 7 comments
Labels
Milestone

Comments

@josephsl
Copy link
Collaborator

Hi,

Continuing the journey toward Python 3:

Is your feature request related to a problem? Please describe.

When walking through a dictionary, Python 2 provides two iteration methods, whereas Python 3 only provides one. Specifically, Python 2 provides a method to return items/keys/values as a list or as an iterator, whereas Python 3 exposes the latter only.

Different parts of NVDA rely on behavioral differences between these two methods. For example, when looking up keys for input management (inputCore), iteritems is used, whereas when looking up voice names in synthesizers (part of synth settings ring), a list of values is used.

Describe the solution you'd like

There are two solutions:

  1. Native/wrappers: for any code fragment that requires a list of dictionary items/keys/values, wrap it around a list call. This means dict.items/keys/values will be strictly for iteration.
  2. Six: use methods provided by this shim to return list/iterator of items/keys/values as appropriate.

Describe alternatives you've considered

None

Additional context

Not only this affects NVDA Core, parts of add-ons that iterate through a dictionary must be rewritten to handle behavior differences between Python 2 and 3. Add-ons community will be encouraged to adopt the solution from Core once a pull request for this item is merged into master branch.

Thanks.

@josephsl josephsl added the z Python 3 transition (archived) Python 3 transition label Dec 14, 2018
@josephsl
Copy link
Collaborator Author

Hi,

Additional notes:

  1. At this point, I recommend adopting the Six module method with a note that it should be ported to native solution after evaluating python 3 transition work.
  2. The pull request regarding this feature won't apply to config.conf (Config Manager) because dict.iteritems and dict.items are used for different purposes unless @LeonarddeR says it can be safely merged somehow.

Thanks.

@josephsl
Copy link
Collaborator Author

Hi,

Actually, upon further reading, it appears Six module does not have an equivalent of dict.items/keys/values for Python 2 (returning lists). So it appears this must be one of those issues that should be solved as part of a master pull request that switches everything to Python 3 at once; this changes if we can find a workaround that preserves compatibility for a while.

Thanks.

@LeonarddeR
Copy link
Collaborator

Actually, upon further reading, it appears Six module does not have an equivalent of dict.items/keys/values for Python 2 (returning lists). So it appears this must be one of those issues that should be solved as part of a master pull request that switches everything to Python 3 at once; this changes if we can find a workaround that preserves compatibility for a while.

There is a pretty straight forward workaround.

Instead of dict.iteritems(), use six.iteritems(dict)
Instead of dict.items(), use list(dict.items())
The latter will result into a list in python 3. In python 2, it will just pass a list to the constructor of list, which copies it. Not very efficient, but it will certainly suffice.
Alternatively, we can just use list(six.iteritems(dict))

2. The pull request regarding this feature won't apply to config.conf (Config Manager) because dict.iteritems and dict.items are used for different purposes unless @leonardder says it can be safely merged somehow.

I'm not sure what you mean here. dict.items() isn't used within the config manager. iteritems is defined within AggregatedSection, however since AggregatedSection isn't a dictionary by itself, you can safely rename that iteritems methods to items, I think. Something that ought to be tested, though.

@josephsl
Copy link
Collaborator Author

josephsl commented Dec 14, 2018 via email

@josephsl
Copy link
Collaborator Author

josephsl commented May 6, 2019

Hi,

In the end, native methods will be used with config manager untouched.

Thanks.

josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 7, 2019
@josephsl
Copy link
Collaborator Author

Hi,

For future reference: whenever one changes iter* to ones without, make sure to check the type of the class where the method is defined in (hence skipping ConfigManager class). Also, staring from today's commits, explanatory comments will be added (see #7105 comment from Reef for details).

Thanks.

josephsl added a commit to josephsl/nvda that referenced this issue May 24, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 24, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 24, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 24, 2019
…cess#7105.

Comment from Reef Turner (NV Access): add explanatory comment, prefixing the comment with '# Py3 review required'. Because some changes are based on another issue (nvaccess#9067, for example), prefix the review comment with issue number references as appropriate.
josephsl added a commit to josephsl/nvda that referenced this issue May 25, 2019
josephsl added a commit to josephsl/nvda that referenced this issue May 25, 2019
…on 3: wrap dict.keys inside a list call along iwth some explanantory comments. Re nvaccess#9067.
josephsl added a commit to josephsl/nvda that referenced this issue May 25, 2019
…hon 3: add explanatory comment on dict.values, along with wrapping one function inside a list call. Re nvaccess#9067.
josephsl added a commit to josephsl/nvda that referenced this issue May 30, 2019
…t call. Re nvaccess#9067.

There are some parts of the source code that uses dict.items/keys/values. With conversion to Python 3, they will need to be wrapped inside a list call because of iteration versus list retrieval differences. These include command sequence in speech manager, UIA event keys, and others.
josephsl added a commit to josephsl/nvda that referenced this issue May 31, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
…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).
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
josephsl added a commit to josephsl/nvda that referenced this issue Jun 4, 2019
michaelDCurran pushed a commit that referenced this issue Jun 8, 2019
…on 3 (#9671)

* Various modules/Python 3: wrap dict.items/keys/values inside a list call. Re #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).

* NVDAObjects/Python 3: dict.iteritems -> dict.items. Re #9067.

* IAccessible handler/Python 3: dict.iteritems -> dict.items. Re #9067.

* UIA handler and utilities/Python 3: dict.iteritems -> dict.items, dict.iterkeys -> dict.keys. Re #9067.

* Add-on handler/Python 3: dict.itervalues -> dict.values. Re #9067.

* App module handler/Python 3: dict.iteritems -> dict.items, dict.itervalues -> dict.values. Re #9067.

* Base object/Python 3: dict.iteritems -> dict.items. Re #9067.

* Braille display detection/Python 3: dict.iteritems -> dict.items. Re #9067

* Braille display drivers/Python 3: dict.iteritems -> dict.items, dict.itervalues -> dict.values. Re #9067.

* Character processing/Python 3: dict.iteritems -> dict.items, dict.itervalues -> dict.values. Re #9067.

* Config/Python 3: dict.iteritems -> dict.items. Re #9067.

* Global plugin handler/Python 3: dict.iteritems -> dict.items. Re #9067.

* GUI/Python 3: dict.iteritems -> dict.items, dict.itervalues -> dict.values. Re #9067.

* Input core/Python 3: dict.iteritems -> dict.items. Re #9067.

* Installer/Python 3: dict.iteritems -> dict.items. Re #9067.

* Oleacc/Python 3: dict.iteritems -> dict.items. Re #9067.

* Speech/Python 3: dict.iteritems -> dict.items. Re #9067.

* Speech XML/Python 3: dict.iteritems -> dict.items. Re #9067.

* Synth drivers/Python 3: dict.iteritems -> dict.items, dict.itervalues -> dict.values. Re #9067.

* Table utils/Python 3: dict.iteritems -> dict.items. Re #9067.

* Text infos offsets/Python 3: dict.iteritems -> dict.items. Re #9067.

* Touch tracker/Python 3: dict.itervalues -> dict.values. Re #9067.

* VK codes/Python 3: dict.iteritems -> dict.items. Re #9067.

* Watchdog/Python 3: dict.iteritems -> dict.items. Re #9067.
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jun 8, 2019
@LeonarddeR
Copy link
Collaborator

Fixed in #9671

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants