Skip to content

Python 3: print statement -> print function - #9664

Merged
michaelDCurran merged 3 commits into
nvaccess:threshold_py3_stagingfrom
josephsl:py3printFunction
Jun 8, 2019
Merged

Python 3: print statement -> print function#9664
michaelDCurran merged 3 commits into
nvaccess:threshold_py3_stagingfrom
josephsl:py3printFunction

Conversation

@josephsl

@josephsl josephsl commented Jun 2, 2019

Copy link
Copy Markdown
Contributor

Link to issue number:

None

Summary of the issue:

In python 3, print keyword is now a function.

Description of how this pull request fixes the issue:

Changes occurrence of "print" statement to a function. Note that this one is also applicable to Python 2, but it is based on Project Threshold/Python 3 assumptions.

Testing performed:

Tested multiple times on source copy of Python 3 NVDA, Python 2 and 3 interpreter, and current NVDA alpha release.

Known issues with pull request:

None

Change log entry:

None

@josephsl josephsl added the z Python 3 transition (archived) Python 3 transition label Jun 2, 2019
@josephsl
josephsl requested a review from feerrenrut June 2, 2019 22:30
@josephsl
josephsl force-pushed the py3printFunction branch from 4559d9b to 808e039 Compare June 4, 2019 16:41
@josephsl
josephsl changed the base branch from threshold to master June 4, 2019 16:55
Comment thread source/synthDrivers/sapi5.py Outdated
hook=NVDAHelper.localLib.dllImportTableHooks_hookSingle(targetDll,importDll,funcName,newFunction)
if hook:
print "hooked %s"%funcName
print("hooked %s"%funcName)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, the print statements in this file should have always been log.debug calls. Currently they are just noise going to stdout.

@josephsl

josephsl commented Jun 4, 2019 via email

Copy link
Copy Markdown
Contributor Author

@michaelDCurran michaelDCurran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer this to go straight to threshold rather than master, unless @feerrenrut instructed otherwise?
Reasoning is that although

print("hello")

Does syntactitally work in Python 2, this is more just a side afect due to the print statement allowing no whitespace between the keyword and a following tuple.
If you try to do things like provide multiple arguments, you see that it literally prints a tuple, and if you try giving it Python 3 keyword arguments, you get a syntax error.
So I think adding it to master now is miss-leading.
Of course in Python 2 we could do:

from __future__ import print_function

But we'll then have to remove that line as soon as threshold is merged to master anyway.

@josephsl

josephsl commented Jun 5, 2019 via email

Copy link
Copy Markdown
Contributor Author

josephsl added 3 commits June 4, 2019 17:09
Reviewed and clarified by Mick Curran (NV Access): print function should have been a log.debug call.
@josephsl
josephsl force-pushed the py3printFunction branch from 7d9fc3f to 1d2aac4 Compare June 5, 2019 00:10
@josephsl
josephsl changed the base branch from master to threshold June 5, 2019 00:10
@josephsl

josephsl commented Jun 5, 2019

Copy link
Copy Markdown
Contributor Author

Hi,

Done - sorry for rebases and forced commits.

Thanks.

@LeonarddeR

LeonarddeR commented Jun 5, 2019 via email

Copy link
Copy Markdown
Collaborator

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, also on this PR, please outline the process used.

@josephsl

josephsl commented Jun 5, 2019

Copy link
Copy Markdown
Contributor Author

Hi,

Process used:

  1. Did grep on print statement (grep -lr "print " source).
  2. For each occurrence, changed the statement to a function.
  3. For SAPI5 module, changed from print statement to a log.debug function.

Thanks.

@michaelDCurran
michaelDCurran changed the base branch from threshold to threshold_py3_staging June 8, 2019 03:24
@michaelDCurran
michaelDCurran merged commit b63554c into nvaccess:threshold_py3_staging Jun 8, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jun 8, 2019
@josephsl
josephsl deleted the py3printFunction branch September 16, 2019 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants