Add type annotations to the module and the unit tests#6
Conversation
|
@ilanbiala: with this, it should be possible to implement full type hinting support for the DBC submodule of cantools... |
|
updated python version tested in CI to 3.10 and 3.14. (python 3.8 has been EOL for a while.) |
andlaus
force-pushed
the
add_type_annotations
branch
from
c3d76d3 to
2ddd392
Compare
|
fun fact: github "simplifies" the |
|
It’s because 3.10 is a number, both in yaml and json. It’s a bit silly that GitHub accepts anything but strings :) |
|
@andlaus is it possible to split out the Python version bump changes into a separate PR and merge that first to isolate this work? |
|
Hey @eerimoq! Sorry to bring this up here, but would it be possible for you to give @andlaus, @zariiii9003, myself, and others more access to the cantools repo and for the readthedocs site for Cantools as well (cantools/cantools#746)? |
yes, but the version bump needs to be done before adding the type hints. be aware that the minimum python version is currently not codified in |
what exactly? |
andlaus
force-pushed
the
add_type_annotations
branch
from
2ddd392 to
a45598c
Compare
|
@eerimoq the Cantools readthedocs site is stuck on a commit that's 2.5 years old and a few version releases out of date (commit revision is at the bottom of the docs site). It looks like permissions haven't been set up correctly for other people to push to the docs site. |
For some of these I am not 100% sure if they are correct. A lot of the code churn of this patch stems from the fact that some classes had to be reordered because python bails out if annotations use classes that are only defined later in the file. This patch features a few minor functional changes which were neccessiated by type annotations: - Support for python2 is no longer declared in `setup.py` - The `Parser.grammar()` methods are supposed to return `Grammar` objects instead of a `Pattern` object which gets implicitly converted to `Grammar` in `Parser.parse()`. IMO this makes the whole affair much less confusing and user code only needs to return `Grammar(x)` instead of `x`. If `grammar()` returns something else, the old behavior is fallen back to, but this will lead to `mypy` complaints in user code... - The `_Token` class is converted from `namedtuple` to a dataclass because named tuples do not play nicely with type annotations and the performance is basically identical. - The internal `_String` class now uses `Pattern` as its base class. (It already had the same API as `Pattern`, but not deriving `_String` from Pattern would necessiate `_String` to be handled separately anyway.) Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com> Approved-by: Christian Hackenbeck <christian.hackenbeck@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
a45598c to
79df0af
Compare
|
ok, I rebased this on top of the latest master version, i.e., no need to fiddle with the python versions anymore... |
|
@andlaus could you update the gha config to run mypy in CI? |
|
good idea. I also added a ruff linting step. |
thanks to [at]zariiii9003 for the suggestion! Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
also fix the complaint about the unused import of the `typing` module. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
724b63c to
64b4204
Compare
|
@ilanbiala, @zariiii9003: any objections to merging this? |
I guess you have a mail in spam folder? @andlaus 
|
thanks to [at]zariiii9003! Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
|
indeed. thanks! |
it probably works, but I'm not completely sure. thanks to [at]zariiii9003 for pointing this out. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
84a6325 to
7a756d3
Compare
- replace `setup.py`, `MANIFEST.in`, and `requirements.txt` with `pyproject.toml` - move the source file to the `textparser` sub-directory and rename it to `__init__.py` the main motivation for this is that I did not get the `py.typed` file included in the wheel with the existing build system... Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
it now uses the same options as cantools which made it quite a bit stricter. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
7a756d3 to
0308fc6
Compare
… optional dependencies group as I see it, "all dependencies not needed by the main module" means "all currently declared dependencies"... Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
7a1bbb8 to
8101941
Compare
…uptools_scm` Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
this is faster than using `isinstance()`. The trick here is to convince type checkers to narrow the type of a match object after `if mo is MISMATCH` statements. The trick here is to make the `_Mismatch` class an enum with `MISMATCH` as its sole member and declare a Literal type for this. thanks to [at]zariiii9003 for comming up with this crazy trick! Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
8101941 to
5697c61
Compare
|
cool. let's see if the automated release process works... |
|
it does. nice :) |
Now release a patch 😄 |
|
@andlaus you lost py.typed in the modernization commit, i missed that... |
|
I think it is a good idea if you tagged a release just for fun ;) |
hm, it seems to work on my machine: |
|
The file itself was deleted (or not commited after moving). I triggered v0.26.1 |
|
cool, thanks! (I forgot to re-add it to git after the move) |
4 participants
For some of these I am not 100% sure if they are correct.
A lot of the code churn of this patch stems from the fact that some classes had to be reordered because python bails out if annotations use classes that are only defined later in the file.
This patch features a few minor functional changes which were neccessiated by type annotations:
setup.pyParser.grammar()methods always returnGrammarobjects instead of aPatternobject which gets implicitly converted toGrammarinParser.parse(). (IMO this makes the whole affair much less confusing and user code only needs to returnGrammar(x)instead ofx.)_Tokenclass is converted fromnamedtupleto a dataclass because named tuples do not play nicely with type annotations and the performance is basically identical._Stringclass now usesPatternas its base class. (It already had the same API asPattern, but not deriving_Stringfrom Pattern would necessiate_Stringto be handled separately anyway.)