Closed mjpieters closed 10 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
34d73e3
) 91.56% compared to head (589325a
) 91.72%. Report is 3 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I managed to completely miss the other PR here, #123. I'll take a look to see what is different between mine and that one.
The difference between this PR and #123 is that this PR is much more limited in scope. It only aims to provide type annotations for consumers of the library, not for the library itself.
See it as a multi-step process; type hinting for internal use could be added separately. This makes this PR a lot smaller.
@mjpieters I will likely merge this PR first and then rebase the other one on your changes. Could you address/resolve these review comments first, though?
The package now includes a PEP 561
py.typed
flag file to let type checkers know that this package provides type annotations.The annotations provide full coverage for all public functions and classes in the aocd package, with the library reaching 100% coverage as per
pyright --ignoreexternal --verifytypes
.A few notes on this PR:
I tried to follow the current best practices for type annotations as much as possible. This includes using
from __future__ import annotations
so the newer but much more readableA | B
union syntax can be used and have the project still work on Python version 3.9. The exception are the type aliases inaocd.types
; Python 3.9 has notyping.TypeAlias
and you can't use|
syntax in a type alias assignment in 3.9 (it's not an annotation). The first issue could be solved by adding a dependency on thetyping_extensions
library, but that seems overkill for one missingtyping
feature.The PR is limited to the public API members only. All private names are left untouched. The goal was to provide consumers of aocd with type annotations, not to validate the types used within aocd itself. Contrast this with #123 which tries to provide full typing coverage of the whole codebase.
I re-introduced the
__all__
list inaocd/__init__.py
because that's what type checkers use to determine what is public more than anything else. Without this list, none of the imports in__init__.py
would be seen as exported. I made sure to only export what was exported before.A new
aocd.types
module holds public type aliases andTypedDict
objects for types that I think consumers ofaocd
might find useful; e.g. a consumer might want to store a returnedPuzzleStats
value, and a 3rd-party library might want to expose a method that passes through aPuzzlePart
andAnswerValue
argument toaocd.submit()
. Plus, these provide inline documentation in IDEs that make use of type information (e.g. PyCharm or Visual Studio Code with the Pylance extension).The
aocd.examples.Page
dataclass andaocd.examples.Example
named tuple already used type annotations for their fields, but these did not properly annotate the optional fields (those that can beNone
as well as their intended type). Changing these to... | None
means their API has now changed. I don't think this is a big deal, it was really a bug and so I doubt this needs a deprecation warning.A second commit extends the Github workflow to validate that the public API keeps the 100% coverage. I recommend adding a
pre-commit
configuration that checks for this locally on every commit. See https://github.com/RobertCraigie/pyright-python#pre-commit for what that looks like.Resolves #78