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

Fix up speechDictVars deprecation #15048

Merged
merged 2 commits into from
Jun 27, 2023
Merged

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Jun 23, 2023

Link to issue number:

Follow up of #15021

Summary of the issue:

The deprecation was incorrectly documented, missing the module name and referring to NVDAState.WritePaths.speechDictsDir instead of WritePaths.speechDictsDir.

The change also caused some issues for add-ons who were relying on the deprecated code being imported into a different module.

Description of user facing changes

N/A

Description of development approach

For ease of use, compatibility for the imported code has been retained, despite not being an API breaking change.

Documentation of the deprecation has been fixed.

Testing strategy:

Known issues with pull request:

None

Change log entries:

Refer to diff

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd requested a review from a team as a code owner June 23, 2023 01:46
@seanbudd seanbudd requested a review from michaelDCurran June 23, 2023 01:46
@seanbudd
Copy link
Member Author

seanbudd commented Jun 23, 2023

cc @ruifontes - I assume this fixes your issue?

@ultrasound1372 - does this also fix the issue you were having? Without more information about what broke for your add-on, we are unable to address your comments in #15021 (comment)

@ruifontes
Copy link
Contributor

For me is good!

@ultrasound1372
Copy link

Apologies for the lack of details, it turns out the paths are slightly different. Here are the errors.
Upon loading NVDA and attempting to load add-ons, application dictionary, Rui's add-on, gives this.

ERROR - globalPluginHandler.listPlugins (21:34:51.307) - MainThread (22376):
Error importing global plugin applicationDictionary
Traceback (most recent call last):
  File "globalPluginHandler.pyc", line 23, in listPlugins
  File "importlib\__init__.pyc", line 127, in import_module
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Users\Colton\AppData\Roaming\nvda\addons\applicationDictionary\globalPlugins\applicationDictionary\__init__.py", line 82, in <module>
    appDictsPath = os.path.abspath(os.path.join(speechDictHandler.speechDictsPath, "appDicts"))
AttributeError: module 'speechDictHandler' has no attribute 'speechDictsPath'

And upon switching config profiles, enhanced dictionaries, which I got from an external source and is available here, gives this.

ERROR - extensionPoints.Action.notify (21:35:12.891) - MainThread (22376):
Error running handler <function _handlePostConfigProfileSwitch at 0x17975300> for <extensionPoints.Action object at 0x02F23510>
Traceback (most recent call last):
  File "extensionPoints\__init__.pyc", line 55, in notify
  File "extensionPoints\util.pyc", line 216, in callWithSupportedKwargs
  File "C:\Users\Colton\AppData\Roaming\nvda\addons\EnhancedDictionaries\globalPlugins\EnhancedDictionaries\__init__.py", line 18, in _handlePostConfigProfileSwitch
    dictHelper.reloadDictionaries()
  File "C:\Users\Colton\AppData\Roaming\nvda\addons\EnhancedDictionaries\globalPlugins\EnhancedDictionaries\dictHelper.py", line 40, in reloadDictionaries
    loadProfileDict()
  File "C:\Users\Colton\AppData\Roaming\nvda\addons\EnhancedDictionaries\globalPlugins\EnhancedDictionaries\dictHelper.py", line 93, in loadProfileDict
    if _hasDictionaryProfile(profile.name, "default.dic"):
  File "C:\Users\Colton\AppData\Roaming\nvda\addons\EnhancedDictionaries\globalPlugins\EnhancedDictionaries\dictHelper.py", line 128, in _hasDictionaryProfile
    return os.path.exists(os.path.join(dictFormatUpgrade.speechDictsPath, profileName or "", dictionaryName))
AttributeError: module 'speechDictHandler.dictFormatUpgrade' has no attribute 'speechDictsPath'

Seems strange that the second one attempts to be looking for the same path in a different place? I'm unsure how to actually test if this PR works myself without it being merged.

@seanbudd
Copy link
Member Author

speechDictsPath really should never have been imported from dictFormatUpgrade. I'd strongly encouraging reaching out to this add-on author to fix the problem from their side.

@seanbudd
Copy link
Member Author

I've pushed further changes to catch this - but I imagine that other issues like this could arise from add-ons. It is really important to import code from the correct origin.

@AppVeyorBot
Copy link

See test results for failed build of commit 4f97fc8dc9

@seanbudd seanbudd merged commit 3f89b60 into master Jun 27, 2023
@seanbudd seanbudd deleted the improveSpeechDictDeprecation branch June 27, 2023 02:10
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants