Python3: fix remaining syntax errors found by py2exe by michaelDCurran · Pull Request #9719 · nvaccess/nvda

michaelDCurran · 2019-06-12T00:12:37Z

Link to issue number:

None.

Summary of the issue:

When running py2exe over the codebase, it found some syntax errors and one missing module.

Description of how this pull request fixes the issue:

Fixes all remaining syntax errors:

tuples in a for loop need to be in brackets.
One file had inconsistant whitepsace where there was a space at the beginning of a line before tabbed indentation.
numbers no longer need or can have an 'L' suffix at the end.
the 'ur' string prefix is no longer supported. Use 'r' instead. Note this was taken from draft pr Python 3: 'ur' string prefixes no longer supported. #9636 plus further strings missed by that pr.
Several strings containing backslashes did not have an 'r' prefix when they should have.
collections.Sequence is now collections.abc.Sequence.
raise excType,value,traceback is no longer valid syntax. Changed the one place we did this to just hold and raise the original exception object, as Python3 now includes the traceback on the exception object itself.

Testing performed:

Py2exe successfully parsed all modules in the codebase.

Known issues with pull request:

Py2exe still fails later on, unrelated to these syntax errors. See my next comment.

Change log entry:

None.

… numbers.

Modified 'ur' strings to be just 'r' strings prefixes

michaelDCurran · 2019-06-12T00:19:24Z

Py2exe still fails later on when copying py2exe/pywintypes37.dll.
It throws an assertion error because although the module has a __file__, attribute, its file extension is not one of the accepted python extensions. Rather it is .dll.
I'm not sure if this is a bug in py2exe... @LeonarddeR or @albertosottile ?
I saw that it was pywintypes37.dll by adding

print(mod.__file__)

directly above the assertion.
Full traceback follows:

Traceback (most recent call last):
  File "setup.py", line 231, in <module>
    + getRecursiveDataFiles('documentation', '../user_docs', excludes=('*.t2t', '*.t2tconf', '*/developerGuide.*'))
  File "C:\Program Files (x86)\Python37-32\lib\site-packages\setuptools\__init__.py", line 145, in setup
    return distutils.core.setup(**attrs)
  File "C:\Program Files (x86)\Python37-32\lib\distutils\core.py", line 148, in setup
    dist.run_commands()
  File "C:\Program Files (x86)\Python37-32\lib\distutils\dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "C:\Program Files (x86)\Python37-32\lib\distutils\dist.py", line 985, in run_command
    cmd_obj.run()
  File "setup.py", line 104, in run
    super(py2exe, self).run()
  File "C:\Users\mick\programming\git\nvda\include\py2exe\py2exe\distutils_buildexe.py", line 192, in run
    self._run()
  File "C:\Users\mick\programming\git\nvda\include\py2exe\py2exe\distutils_buildexe.py", line 273, in _run
    builder.build()
  File "C:\Users\mick\programming\git\nvda\include\py2exe\py2exe\runtime.py", line 264, in build
    self.copy_files(destdir)
  File "C:\Users\mick\programming\git\nvda\include\py2exe\py2exe\runtime.py", line 508, in copy_files
    assert mod.__file__.endswith(EXTENSION_SUFFIXES[0])
AssertionError

LeonarddeR · 2019-06-12T03:59:16Z

In my opinion, the actual bug here is that py2exe tries to copy stuff for pywin32, but we don't use pywin32 anymore within our code base. So rather than coming up with a fix or workaround for the assertion, we should try to stop copying parts of pywin32 altogether.

LeonarddeR · 2019-06-12T04:52:49Z

See #9711 and #9701

LeonarddeR · 2019-06-12T04:54:56Z

 		oldTime=getattr(self,'_basicTextTime',0)
 		if newTime-oldTime>0.5:
-			self._basicText=u" ".join([x for x in self.name, self.value, self.description if isinstance(x, basestring) and len(x) > 0 and not x.isspace()])
+			self._basicText=u" ".join([x for x in (self.name, self.value, self.description) if isinstance(x, basestring) and len(x) > 0 and not x.isspace()])


The list (square) brackets were actually a part of this old syntax and can be removed. Also note that basestring is still in here, but that will probably be handled in a later stage?

Suggested change

self._basicText=u" ".join([x for x in (self.name, self.value, self.description) if isinstance(x, basestring) and len(x) > 0 and not x.isspace()])

self._basicText=u" ".join(x for x in (self.name, self.value, self.description) if isinstance(x, basestring) and len(x) > 0 and not x.isspace())

LeonarddeR · 2019-06-12T04:55:50Z

 			d=dict.get('_del_%s'%x,None)
 			if x in dict:
-				methodsString=",".join([str(i) for i in g,s,d if i])
+				methodsString=",".join([str(i) for i in (g,s,d) if i])


Square brackets can be removed

LeonarddeR · 2019-06-12T04:58:06Z

 				if className:
 					break
-	return ".".join([x for x in path,className,funcName if x])
+	return ".".join([x for x in (path,className,funcName) if x])


brackets can be removed

LeonarddeR · 2019-06-12T04:59:06Z

@@ -0,0 +1,18 @@
+import sys


Interesting stuff ;)

LeonarddeR · 2019-06-12T04:59:37Z

 		exc = self._exc_info
 		if exc:
-			raise exc[0], exc[1], exc[2]
+			# The execution of the function in the other thread cuased an exception.


cuased > caused

LeonarddeR · 2019-06-12T05:02:34Z

+			# Re-raise it here.
+			# Note that in Python3, the traceback (stack) is now part of the exception,
+			# So the logged traceback will correctly show the stack for both this thread and the other thread. 
+			raise e


Where is this e coming from? Shouldn't this be exc?

michaelDCurran · 2019-06-12T05:31:46Z

@LeonarddeR I believe I have addressed your review comments. Changes of basestring etc are coming in a separate pr very soon.
I have removed the accidentally committed test_traceback.py ;) but at least you got to see how I confirmed how Python3 exceptions work.

…pywintypes37.dll.

michaelDCurran · 2019-06-12T05:44:57Z

setup.py now excludes win32api (which we don't need), so it gets around the possible bug when trying to include pywintypes37.dll.

LeonarddeR · 2019-06-12T06:01:42Z

+			"serial.serialposix", 
+			"serial.socket_connection",
+			# Py2exe seems to implicitly include this, but we don't need it. 
+			"win32api",


I'd really like to know why this was included.

Also note the following lines from the output:

Copy d:\Development\NVDA\source repo\include\py2exe\win32wnet.pyd to D:\Development\NVDA\source repo\dist\win32wnet.pyd Copy d:\Development\NVDA\source repo\include\py2exe\_winxptheme.pyd to D:\Development\NVDA\source repo\dist\_winxptheme.pyd Copy ExtensionDLL d:\Development\NVDA\source repo\include\py2exe\pywintypes37.dll to D:\Development\NVDA\source repo\dist\

These 3 files are all pywin32. We also have the following files in library.zip:

win32wnet.pyc

winxptheme.pyc

_winxptheme.pyc

LeonarddeR · 2019-06-12T06:08:57Z

Small additional detail. A non public application I wrote and converted to py2exe lately does not contain all these modules. So I really have the feeling that there is a reference in our code we aren't currently aware of.

michaelDCurran · 2019-06-12T07:13:48Z

If I manually delete netbios.py, win32api.pyd, win32wnet.pyd and winxptheme.py from our py2exe git submodule, Py2exe then tells me the following modules are missing and why: ? netbios imported from uuid ? winxptheme imported from wx.lib.agw.aui.dockart, wx.lib.agw.aui.framemanager ? winxptheme imported from wx.lib.agw.aui.dockart, wx.lib.agw.aui.framemanager I have confirmed online that winxptheme (from pywin32 is used in wxPython if available. I can also see from Python3's own uuid module source code, that to generate a uuid, it tries various different ways of generating a hardware node value, one of those ways is to use netbios / win32wnet if available. In short, we should be safe to exclude winxptheme, netbios and win32wnet. However, @LeonarddeR, what was the reasoning behind why you thought that py2exe depended upon pywin32? I assume it was something more significant than missing modules at this stage in the build? I.e. knowing this now, should we just put these in excludes, or can we actually remove these modules from the git submodule?

…not needed.

michaelDCurran and others added 2 commits June 12, 2019 08:38

Bracket some tuples in for loops, and remove L (long-type)suffix from…

bdd78ff

… numbers.

Python 3: 'ur' string prefixes no longer supported.

8250a95

Modified 'ur' strings to be just 'r' strings prefixes

michaelDCurran requested review from LeonarddeR and feerrenrut June 12, 2019 00:12

More syntax fixes.

f9ddb13

michaelDCurran force-pushed the py3_syntax branch from 17dcd48 to f9ddb13 Compare June 12, 2019 00:27

LeonarddeR suggested changes Jun 12, 2019

View reviewed changes

Address review comments.

f34b294

setup.py: exclude win32api which stops py2exe from trying to include …

28e3c6a

…pywintypes37.dll.

LeonarddeR reviewed Jun 12, 2019

View reviewed changes

LeonarddeR approved these changes Jun 12, 2019

View reviewed changes

setup.py: exclude netbios and winxptheme pywin32 modules as they are …

bf4f8b1

…not needed.

michaelDCurran merged commit 1164178 into threshold_py3_staging Jun 12, 2019

nvaccessAuto added this to the 2019.3 milestone Jun 12, 2019

This was referenced Jun 12, 2019

Remove trailing 'I' or 'L' from number literals. #9701

Closed

Python version 3.7 does not support a 'ur' prefix #8660

Closed

josephsl mentioned this pull request Jul 23, 2019

What's new and readme: we are moving to Python 3.7 #9942

Merged

	self._basicText=u" ".join([x for x in (self.name, self.value, self.description) if isinstance(x, basestring) and len(x) > 0 and not x.isspace()])
	self._basicText=u" ".join(x for x in (self.name, self.value, self.description) if isinstance(x, basestring) and len(x) > 0 and not x.isspace())

Uh oh!

Uh oh!

Conversation

michaelDCurran commented Jun 12, 2019

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

michaelDCurran commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LeonarddeR commented Jun 12, 2019 via email

Uh oh!

LeonarddeR commented Jun 12, 2019

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

michaelDCurran commented Jun 12, 2019

Uh oh!

michaelDCurran commented Jun 12, 2019

Uh oh!

LeonarddeR Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Jun 12, 2019

Uh oh!

michaelDCurran commented Jun 12, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

michaelDCurran commented Jun 12, 2019 •

edited

Loading