Skip to content

Add type annotations to the module and the unit tests#6

Open
andlaus wants to merge 2 commits into
cantools:masterfrom
andlaus:add_type_annotations
Open

Add type annotations to the module and the unit tests#6
andlaus wants to merge 2 commits into
cantools:masterfrom
andlaus:add_type_annotations

Conversation

@andlaus

@andlaus andlaus commented Jun 8, 2026

Copy link
Copy Markdown
Member

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 always 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.)
  • 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.)

@andlaus andlaus requested a review from zariiii9003 June 8, 2026 11:27
@andlaus

andlaus commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

@ilanbiala: with this, it should be possible to implement full type hinting support for the DBC submodule of cantools...

@andlaus

andlaus commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

updated python version tested in CI to 3.10 and 3.14. (python 3.8 has been EOL for a while.)

@andlaus andlaus force-pushed the add_type_annotations branch from c3d76d3 to 2ddd392 Compare June 8, 2026 11:38
@andlaus

andlaus commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

fun fact: github "simplifies" the 3.10 version specifier to 3.1. adding quotes around the versions fixes that...

@eerimoq

eerimoq commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

It’s because 3.10 is a number, both in yaml and json. It’s a bit silly that GitHub accepts anything but strings :)

@ilanbiala

Copy link
Copy Markdown
Member

@andlaus is it possible to split out the Python version bump changes into a separate PR and merge that first to isolate this work?

@ilanbiala

Copy link
Copy Markdown
Member

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)?

@andlaus

andlaus commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

@andlaus is it possible to split out the Python version bump changes into a separate PR and merge that first to isolate this work?

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 setup.py: setup.py only specifies the Programming Language :: Python :: {2,3} (informal) classifiers and the yaml for github action mentions python 3.8 and 3.13 a few times, i.e., it is currently tested on python 3.8 and 3.14 (but not 2.x) and there's no formal dependency...

@eerimoq

eerimoq commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

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)?

what exactly?

- drop Python2 classifier
- specify python 3.10 as mininmum python version
- update github action config file to use python 3.10 and 3.14

Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
Approved-by: Michael Hahn <michael.a.hahn@mercedes-benz.com>
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 andlaus force-pushed the add_type_annotations branch from 2ddd392 to a45598c Compare June 9, 2026 12:45
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.

3 participants