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

Deprecate import of Commands directly from speech module in favour of speech.commands #12126

Merged
merged 6 commits into from
Mar 12, 2021

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Mar 5, 2021

Link to issue number:

Implements deprecation announced in: #11049, #10371, #10885
Part of: #12123

Summary of the issue:

Commands from the module speech.commands were being imported as from .commands import * in speech\__init__.py and speech\manager.py.

As a result, commands were often being imported as import speech; speech.ExampleCommand() or import speech.manager; speech.manager.ExampleCommand().

Description of how this pull request fixes the issue:

The deprecated behaviour is removed and commands must be imported explicitly as from speech.commands import ExampleCommand

Testing strategy:

Check code correctness by the following process. Do the following using an unchanged repository based on the latest master commit (db664be), so code can be compared properly.

Using git bash tools find & sed (may be also included in Windows Subsystem Linux)

[a-zA-Z_][a-zA-Z0-9_]* is the regex for valid python variable name prefixes

# removes "speech.ExampleCommand" usages from python files "ExampleCommand"
find . -type f -name "*.py" -exec sed -i'' -bre 's/speech\.([a-zA-Z_][a-zA-Z0-9_]*Command)/\1/' {} +

# removes "speech.commands.ExampleCommand" usages in favour of "ExampleCommand"
find . -type f -name "*.py" -exec sed -i'' -bre 's/speech\.commands\.([a-zA-Z_][a-zA-Z0-9_]*Command)/\1/' {} +

# removes "speech.manager.ExampleCommand" usages in favour of "ExampleCommand"
# there is no instances of NVDA with this usage, but this is used to ensure that fact considering the code removal
find . -type f -name "*.py" -exec sed -i'' -bre 's/speech\.manager\.([a-zA-Z_][a-zA-Z0-9_]*Command)/\1/' {} +

Do the following to confirm commands aren't being imported directly from speech or speech.manager

find . -type f -name '*.py' -exec grep -P "from +speech +import" {} +
find . -type f -name '*.py' -exec grep -P "from +speech\.manager +import" {} +

Compare the current code to this branch: nvda/dep-11049, note the only changes are:

  • the removed, deprecated code
  • the changed use of imports (hard to do automatically, any issues should be caught by flake8)
  • whitespaces fixes for flake8
git diff origin/dep-11049

Known issues with pull request:

None

Change log entry:

Dev Changes

- Commands cannot be directly imported from speech as `import speech; speech.ExampleCommand()` or `import speech.manager; speech.manager.ExampleCommand()`. Commands should be imported explicitly as `from speech.commands import ExampleCommand` (#12126)

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation. (N/A)
  • Change log entry.
  • Context sensitive help for GUI changes. (N/A)

@AppVeyorBot
Copy link

See test results for failed build of commit 03ac978eab

@seanbudd seanbudd changed the title WIP - Deprecate import of Commands directly from speech module in favour of speech.commands Deprecate import of Commands directly from speech module in favour of speech.commands Mar 9, 2021
@seanbudd seanbudd mentioned this pull request Mar 9, 2021
18 tasks
@seanbudd seanbudd self-assigned this Mar 9, 2021
@seanbudd seanbudd added this to the 2021.1 milestone Mar 9, 2021
@seanbudd seanbudd added the deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release label Mar 9, 2021
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Looks good to me. Nice use of sed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants