Skip to content

nvwave.playWaveFile: async > asynchronous#8647

Merged
michaelDCurran merged 4 commits into
nvaccess:thresholdfrom
josephsl:py3nvwaveAsync
Jun 6, 2019
Merged

nvwave.playWaveFile: async > asynchronous#8647
michaelDCurran merged 4 commits into
nvaccess:thresholdfrom
josephsl:py3nvwaveAsync

Conversation

@josephsl

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #8607

Summary of the issue:

Change async argument in nvwave.playWaveFile to asynchronous.

Description of how this pull request fixes the issue:

Python 3.5 introduces 'async' and 'await' keywords to deal with asynchronous generators and other possibilities. Since Python 3.7, use of these keywords as variable names is no longer allowed. In NVDA code, nvWave.playWaveFile is affected, so rename 'async' to 'asynchronous'.

Testing performed:

Compiled and tested various code paths that calls nvwave.playWaveFile with the new keyword arguments.

Known issues with pull request:

Any add-on that sets "async" argument to True will be affected.

Change log entry:

Changes for developers:
The "async" argument in nvwave.playWaveFile has been renamed to "asynchronous". Add-ons that relied on this name should be modified to use the new argument name. (#8607)

@josephsl josephsl requested a review from LeonarddeR August 19, 2018 04:07
LeonarddeR
LeonarddeR previously approved these changes Aug 20, 2018
Comment thread source/nvwave.py
fileWavePlayerThread=None
def playWaveFile(fileName, async=True):
def playWaveFile(fileName, asynchronous=True):
"""plays a specified wave file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

May be add information about parameters to the doc string while at it?

@josephsl

josephsl commented Aug 20, 2018 via email

Copy link
Copy Markdown
Contributor Author

@michaelDCurran

Copy link
Copy Markdown
Member

Hold off until @feerrenrut has finished add-on versioning.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@michaelDCurran commented on 5 Oct 2018, 09:10 CEST:

Hold off until @feerrenrut has finished add-on versioning.

Think we're good to go now.

@LeonarddeR LeonarddeR changed the base branch from master to threshold April 27, 2019 15:20
@feerrenrut feerrenrut changed the base branch from threshold to master May 13, 2019 17:08
@feerrenrut

Copy link
Copy Markdown
Contributor

I've changed the base branch on this back to master, since it doesn't break compatibility with python 2, or introduce any risky changes there. Merging to master reduces the number of things that may require merging.

Thinking about risk, is it possible for this name change to affect add-ons?

@josephsl

josephsl commented May 13, 2019 via email

Copy link
Copy Markdown
Contributor Author

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@feerrenrut commented on 13 May 2019, 19:11 CEST:

... it doesn't break compatibility with python 2, or introduce any risky changes there. Merging to master reduces the number of things that may require merging.

Note that all code that passes async as a keyword argument will fail after merging this. While I can't think of any add-ons that utilize this code, I'd be very reluctant to merge this into a Python 2 version of NVDA if i wanted to make sure that i wouldn't break things. Note that there's nothing wrong with breaking things like this if we agree that we don't care.

@feerrenrut

Copy link
Copy Markdown
Contributor

I'd be very reluctant to merge this into a Python 2 version of NVDA if i wanted to make sure that i wouldn't break things

Yes, I think this introduces too much unnecessary risk. Python 3 migration is going to break add-ons anyway, so we will direct this risk there. I will re-target this PR.

@feerrenrut feerrenrut changed the base branch from master to threshold May 17, 2019 11:42
@josephsl

josephsl commented May 17, 2019 via email

Copy link
Copy Markdown
Contributor Author

josephsl added 3 commits June 6, 2019 14:59
Python 3.5 introduces 'async' and 'await' keywords to deal with asynchronous generators and other possibiliites. Since Python 3.7, use of these keywords as variable names is no longer allowed. In NVDA code, nvWave.playWaveFile is affected, so rename 'async' to 'asynchronous'.
Reviewed by Leonard de Ruijter (Babbage): document the renamed keyword arg.
@josephsl

josephsl commented Jun 6, 2019

Copy link
Copy Markdown
Contributor Author

Hi,

Conflict resolved and is ready for another round of review.

Steps taken:

  1. Did grep -lr "async" and located instances of nvwave call.
  2. Changed to "asynchronous".

Thanks.

@josephsl josephsl deleted the py3nvwaveAsync branch September 16, 2019 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants