Skip to content

Python 3 imports: try importing Python 3 version before resorting to using Python 2 names#8606

Merged
feerrenrut merged 9 commits into
nvaccess:masterfrom
josephsl:py3imports
Aug 22, 2018
Merged

Python 3 imports: try importing Python 3 version before resorting to using Python 2 names#8606
feerrenrut merged 9 commits into
nvaccess:masterfrom
josephsl:py3imports

Conversation

@josephsl

@josephsl josephsl commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

Hi,

Part of preparation for Python 3 transition:

Link to issue number:

Fixes #8527
Fixes #8590
Fixes #8649

Summary of the issue:

Use Python 3 module names before resorting to using python 2 names.

Description of how this pull request fixes the issue:

Python 3 has renamed several modules. These include _winreg to winreg and Queue to queue. At the moment some NVDA modules import affected modules directly or load one or two things from these. Thus a try/except block has been added to load Python 3 version first before resorting to Python 2 names, and for some, edited various code fragments to use Python 3 names.

Testing performed:

Compiled and ran NVDA from source, along with running Python 2.7 and 3.7 interpreters to make sure names are correct.

Known issues with pull request:

None

Change log entry:

None

Thanks.

@josephsl josephsl requested a review from feerrenrut August 8, 2018 02:01
@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 8 aug. 2018 04:00 CEST:

Testing performed:

Compiled and ran NVDA from source, along with running Python 2.7 and 3.7 interpreters to make sure names are correct.

Could you elaborate on how you've tested that this pr is actually complete with regard to module changes? I.e. what is the expected result when you run your pr code with the python 3.7 interpreter?

@josephsl

josephsl commented Aug 9, 2018

Copy link
Copy Markdown
Contributor Author

Hi,

It did load the modules correctly when I attempted to run NVDA under Python 3.7. I also ran interpreters and attempted to load Python 3 modules.

The other issue is abolute versus relative imports 9extension points, for example), which I'll address in another issue/pull request pair (I'm still looking for these imports at the moment).

Thanks.

Python 3 introduces winreg - just _winreg without the leading underscore. Thus use the new import form:
>>> try:
>>>  import winreg
>>> except ImportError:
>>>  import _winreg as winreg
Note: spaces were used for this fragment.
This change allows NVDA to work with Python 2 or 3 version of winreg module. Also, code that uses (_)winreg module has been modified to call winreg.something.
Python 3 ships with queue (lowercase queue). This module is used in three places: queue handler, JaB handler, and Espeak module. For this commit, queue handler has been modified to import Python 3's queue first before resorting to loading Queue.queue (Python 2). JAB and Espeak will be done on a separate commit due to extensive edits required (these use Python 2 module directly).
Python 3 renames Queue to queue, and internal Espeak module imports this directly. Thus try loading Python 3 version first before resorting to useing Python 2 name.
@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Regarding relative versus absolute imports, I'll address this in this pull request as well.

Thanks.

@feerrenrut

Copy link
Copy Markdown
Contributor

for the blocks that try importing and catch the import error exception, could you please add comments to explain why this is happening. In the context of this PR it's obvious, but without perhaps not. I would suggest something along the lines of:

# module as named in python 2.7
# module name in python 3.7

@josephsl

josephsl commented Aug 20, 2018 via email

Copy link
Copy Markdown
Contributor Author

Commented by Reef Turner (NV Access): comment module renames.
@LeonarddeR LeonarddeR added the z Python 3 transition (archived) Python 3 transition label Aug 21, 2018
… version for performance considerations.

Reviewed by Reef Turner (NV Access): use Python 2 module first, and then use Python 3 if the import machinery fails in consideration for import performance.
@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

@feerrenrut, done as suggested.

For others: the keyword to look for will be "python 3 import". Alternatively, when using grep, use "Python 2.+" (2.x, 2.7, etc.).

Thanks.

@feerrenrut feerrenrut merged commit 8e2df9f into nvaccess:master Aug 22, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.4 milestone Aug 22, 2018
@josephsl josephsl deleted the py3imports branch July 19, 2020 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants