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

Introduce annotations throughout add-on source code #155

Open
josephsl opened this issue Jan 4, 2021 · 2 comments · May be fixed by #164
Open

Introduce annotations throughout add-on source code #155

josephsl opened this issue Jan 4, 2021 · 2 comments · May be fixed by #164

Comments

@josephsl
Copy link
Collaborator

josephsl commented Jan 4, 2021

Hi,

This is more towards new feature/change/maintenance, but will be beneficial for future maintainers:

Python allows annotating types in source code. This is of the form:

something: type = value

For functions:

def function(args: type) -> returnType

This allows developers to understand usage cases for code and is checked by static type checkers such as Mypy. For the SPL add-on, it will allow future maintainers to grasp how functions are used and for which purpose. Annotating add-on code also allows ease of development through IDE's that will perform static type checking.

Note: due to extensive modifications, this change will focus on stable releases i.e. will not be backported to LTS20. Also, because Python 3.9 introduces generic type declarations without importing typing module, a future module import statement will be present on all files with annotations added for forward compatibility (once Python 3.10 or later is in use, future import statement can be removed). To help with annotations, Mypy will be used.

Thanks.

@josephsl
Copy link
Collaborator Author

josephsl commented Jan 4, 2021

Hi,

Note that not all code will be annotated - most notably, class methods will not be annotated for now to avoid regressions when editing code due to Mypy output.

Thanks.

josephsl added a commit that referenced this issue Jan 23, 2021
…rward compatibility. Re #155.

Annotate functions and variables to make it clear as to what is what and what type it is. As a first step, use __future__.annotations import to guarantee forward and backward compatibility (to be removed in Python 3.10 version). This is better than importing uppercase versions of containers from typing module as these are deprecated in Python 3.9.
The overall type annotations are guided by Mypy, a static type checker.
josephsl added a commit that referenced this issue Jan 23, 2021
Mypy: annotate containers with no type information such as dictionaries and lists. This applies to both module-level and class attributes.
josephsl added a commit that referenced this issue Jan 23, 2021
Mypy says app module class definition is ambiguous (no-redef). Therefore add a 'type: ignore' comment and add an explanatory text to accompany it.
josephsl added a commit that referenced this issue Jan 23, 2021
Annotate containers, including find text, cart edit timestamp used in cart explorer, and config profile status map.
josephsl added a commit that referenced this issue Jan 23, 2021
Microphone alarm timer is officially annotated as threading.Timer type.
josephsl added a commit that referenced this issue Jan 23, 2021
Some module-level variables are initialized to None, including library scan monitor thread, SPLConfig itself, and early metadata announcer timer. Therefore annotate these.
josephsl added a commit that referenced this issue Jan 25, 2021
…ds. Re #155.

Annotate Studio app module class methods. Exceptions include those starting with script_*, event_*, constructor, terminate, and overlay class chooser.
josephsl added a commit that referenced this issue Jan 25, 2021
…ross base class and subclasses. Re #155.

Annotate class methods for SPL track item class and derivatives. This mostly concerns indexOf method, and for Studio version, additoinal methods.
josephsl added a commit that referenced this issue Jan 25, 2021
Annotate encoder class methods. Exceptions include name getter and a method to move to different rows.
josephsl added a commit that referenced this issue Jan 25, 2021
Annotate ConfigHub class methods, including constructor. The lone exception is __delitem__ method as it is a custom version of ChainMap's method.
josephsl added a commit that referenced this issue Jan 25, 2021
.

When initializing cart explorer, an empty dictionary can be passed into cart explorer init function. Make sure this is annotated correctly (Optional[dict]).
josephsl added a commit that referenced this issue Jan 25, 2021
josephsl added a commit that referenced this issue Jan 26, 2021
Mypy: re-initialize ChainMap.maps as a list of ConfigObj. This complies with ChainMap specification in that maps is a list of mappings, and ConfigObj, as a dictionary/section, is indeed a mapping. To acknowledge that ConfigHub is powered by ChainMap, call super method, and then re-initialize self.maps. This resolves attribute definition errors from Mypy when accessing ConfigObj attributes.
josephsl added a commit that referenced this issue Jan 26, 2021
Annotate ConfigHub class attributes to better communicate attribute types and to guard against potential bugs due to type misuse.
josephsl added a commit that referenced this issue Jan 26, 2021
…nteger. Re #155.

Mypy: switch flags must be an integer, not None or an integer. This guards against argument and attribute type mismatch.
josephsl added a commit that referenced this issue Jan 26, 2021
Caught by Mypy: return None if Studio is not running to make it consistent with type annotation information.
josephsl added a commit that referenced this issue Jan 26, 2021
Mypy: without annotating snapshot record map inside playlist snapshots method, Mypy will complain about assignment expression issues with type mismatch. Therefore annotate the map itself.
josephsl added a commit that referenced this issue Jan 26, 2021
Mypy: not ideal, but some metadata stream list items can be None, possibly because Studio dies in the middle of retrieving metadata streaming values. Therefore metadata list function will return a list of None and integers.
josephsl added a commit that referenced this issue Jan 27, 2021
… turns out to be None. Re #155.

After openConfig is called, SPLConfig would be something but Mypy assumes config will be None. Therefore to satisfy Mypy and to guard against this scenario, return from initializing ad-on config if SPLConfig does become None. At least this allows Studio app module to load, it may result in errors, which can be a clue to developers that soemthing odd must have happened with ConfigHub.
josephsl added a commit that referenced this issue Jan 27, 2021
…None. Re #155.

Mypy: because Studio API may return None if Studio dies, make sure this is not the case for library scan count and track count as defined in studio monitor.
josephsl added a commit that referenced this issue Jan 27, 2021
… integer) so studio monitor can have correct type information for it. Re #155.
josephsl added a commit that referenced this issue Jan 27, 2021
…(optional integer) to help with integer comparisons. Re #155.

Annotate library scan count variable, which helps with integer comparison. Also, in the midst of a scan loop, if library scan count becomes None, return early (and also set lib scan flag to False in the process).
josephsl added a commit that referenced this issue Jan 27, 2021
… None before entering monitor loop. Re #155.

Before entering library scan monitor loop, make sure library scan count is not None. This is now done earlier so that nitial library scan reporter can exit early if lib scan count is indeed None and also to ensure that subsequent reference to lib scan count is seen as an integer.
josephsl added a commit that referenced this issue Jan 27, 2021
Without annotating place marker, Mypy flags track locator call found in place marker script as type mismatch. Therefore flag place marker as an optoinal string.
josephsl added a commit that referenced this issue Jan 27, 2021
… nothing. Re #155.

Mypy: make sure place marker is neither an empty string nor None because track locator expects a genuine string for search text. This also results in moving the conditionals around in place marker script.
josephsl added a commit that referenced this issue Jan 27, 2021
Mypy: in library scan monitor script, make sure scan count is an integer, and return early if None otherwise.
josephsl added a commit that referenced this issue Jan 27, 2021
Mypy: because time announcer is typically called from time commands after obtaining Studio API results for track duration, make sure to catch the case where time integer can be None, and if yes, return early. This change resolves type mismatch for time commands.
josephsl added a commit that referenced this issue Dec 29, 2023
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
@josephsl
Copy link
Collaborator Author

Hi,

Reopening as NVDA 2024.1 is in beta phase (upgrading to Python 3.11). Therefore, future import can be removed once 2024.1 requirement is fully in effect, at which point this issue will be closed again.

Thanks.

@josephsl josephsl reopened this Dec 29, 2023
josephsl added a commit that referenced this issue Mar 18, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue Mar 25, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue Mar 30, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue May 17, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
@ChrisDuffley ChrisDuffley linked a pull request Oct 3, 2024 that will close this issue
josephsl added a commit that referenced this issue Dec 26, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue Dec 26, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue Dec 26, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
josephsl added a commit that referenced this issue Dec 26, 2024
As NVDA 2024.1 requirement is in effect, there is no need to import __future__ as NVDA is powered by Python 3.11.
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 a pull request may close this issue.

1 participant