Skip to content

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

Merged
seanbudd merged 6 commits into
masterfrom
dep-11049
Mar 12, 2021
Merged

Deprecate import of Commands directly from speech module in favour of speech.commands#12126
seanbudd merged 6 commits into
masterfrom
dep-11049

Conversation

@seanbudd

@seanbudd seanbudd commented Mar 5, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown

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

@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 to me. Nice use of sed!

@seanbudd seanbudd merged commit c901b0f into master Mar 12, 2021
@seanbudd seanbudd deleted the dep-11049 branch March 12, 2021 00:05
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.

3 participants