Closed juhoinkinen closed 1 year ago
Patch coverage: 100.00
% and no project coverage change.
Comparison is base (
4f6994b
) 99.66% compared to head (252c75f
) 99.67%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
I annotated manually one module (annif.backend.hyperopt.py) that monkeytype was not able to process. These annif package modules are still lacking type hints altogether:
Not sure if they all need type hints.
Currently running mypy --ignore-missing-imports
produces 154 errors...
Now I noticed that the hints of the standard collection types do not need to be from the ones from the typing
module, but I could use the built-in types, when the from __future__ import annotations
is used (PEP585).
Also, it could be considered to try using type hints only with Python 3.10. That would allow simpler syntax for Union[] and dropping using Optional[] (PEP604).
Also, it could be considered to try using type hints only with Python 3.10.
Sounds promising, but would the code still run with Python 3.8 and 3.9 if it included type hints using the new syntax?
Also, it could be considered to try using type hints only with Python 3.10.
Sounds promising, but would the code still run with Python 3.8 and 3.9 if it included type hints using the new syntax?
Changing Union[x, y]
to x | y
in few places did not make the unit tests fail on Python 3.8, but did not even affect mypy runs...
I think we should use the simplest, most elegant, most modern approach that works with Python 3.8 (the annotations don't have to work, but the code needs to run). So starting directly with PEP 585 and PEP 604 would make sense if that can be done.
I think we should use the simplest, most elegant, most modern approach that works with Python 3.8 (the annotations don't have to work, but the code needs to run). So starting directly with PEP 585 and PEP 604 would make sense if that can be done.
Using the from __future__ import annotations
postpones the evaluation of annotations, so the annotations "can contain whatever", so PEP 585 and 604 can be used even with Python 3.8.
And it seems mypy also supports the PEP 585 and 604 features/syntax also on Python 3.8 from mypy version 0.800 onwards (when using the mentioned import).
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells
No Coverage information
0.0% Duplication
I think this could be merged now, and with a squash merge. This just adds type hints to function/method signatures and is already touching quite many lines. However, runtime behaviour should not be affected.
There are many mypy errors (see PR description/first comment), but resolving many of them would need modifying actual code (which could then affect runtime, e.g. "X" has no attribute "__iter__" (not iterable)
, Incompatible return value type (got "Union[bool, str, None]", expected "Optional[bool]") [return-value]
), or at least annotating variables inside functions. Another PR could be more appropriate for this.
compiles Python modules to C extensions. It uses standard Python type hints to generate fast code.
The first round of type hints were generated by running
monkeytype apply --pep_563
on all modules.Then type hints have been manually corrected, and simplified by (at least with)
float
forint | float
(allowed by PEP484)dict[X, Any]
for many dictionaries, because otherwise more narrow union types become long and verbose (the type of the key/value cannot be set as union inside the dict, e.g.dict[str, float | str]
does not work, but this should be written asdict[X, str] | dict[X, int]
; )float
fornumpy.float64
andint
fornumpy.int32
etc.I have been using this mypy configuration (in
mypy.ini
), which allows to just runmypy
in root of Annif repo and it makes mypy ignore a bunch of errors:With this config mypy gives "only" 20 errors.
Show errors
When not ignoring any errors, mypy throws 182 of them.
Show all errors
```bash annif/analyzer/analyzer.py:25: error: Skipping analyzing "nltk.tokenize": module is installed, but missing library stubs or py.typed marker [import] annif/analyzer/analyzer.py:25: error: Skipping analyzing "nltk": module is installed, but missing library stubs or py.typed marker [import] annif/openapi/validation.py:7: error: Skipping analyzing "connexion": module is installed, but missing library stubs or py.typed marker [import] annif/openapi/validation.py:8: error: Skipping analyzing "connexion.exceptions": module is installed, but missing library stubs or py.typed marker [import] annif/openapi/validation.py:9: error: Skipping analyzing "connexion.utils": module is installed, but missing library stubs or py.typed marker [import] annif/lexical/tokenset.py:44: error: Need type annotation for "_index" [var-annotated] annif/lexical/tokenset.py:58: error: Need type annotation for "subj_tsets" (hint: "subj_tsets: Dict[Closes #690.