-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add type annotations #603
base: master
Are you sure you want to change the base?
Add type annotations #603
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #603 +/- ##
==========================================
- Coverage 98.93% 89.98% -8.95%
==========================================
Files 6 6
Lines 750 869 +119
==========================================
+ Hits 742 782 +40
- Misses 8 87 +79 ☔ View full report in Codecov by Sentry. |
e0bda54
to
87dfa2e
Compare
a80bea3
to
98aa1ac
Compare
08a02e4
to
d0b821a
Compare
402da6c
to
cd2fcde
Compare
This behavior wasn't documented and didn't fully work: it breaks as soon as you try to save a model with a `None` value.
This allows using the latest annotation syntax supported by the type checker regardless of the runtime Python version.
TODO: Review annotations after code changes made later.
The `Choices` constructor actually only accepts lists and tuples, not arbitrary sequences. However, using `list` in the annotation opens a can of worms related to type variance.
This allows using the latest annotation syntax supported by the type checker regardless of the runtime Python version.
These annotations are sufficient to pass mypy inspection if mypy is configured to allow unannotated functions.
This required a bit of refactoring to get the type of `STATUS` correct for each suite. There are two cases which I decided not to support in the type system: - passing a list instead of a tuple when defining an option group - `in` checks using a data type that doesn't match the choices
We also type check the unit tests and the unit tests now import functionality from pytest.
I can't find a way to inform mypy of the actual types without duplicating a lot of test code.
This preserves the type of the wrapped descriptor (usually a field). Maybe this is overkill, as `DescriptorWrapper` seems to only be used as part of the `FieldTracker` implementation and is not documented and barely tested. But technically, it is public API.
@mthuurne time to merge this and release it as beta? |
Ah sorry, I've been pretty busy lately organizing a conference (shameless plug: T-DOSE) and this escaped my attention. I agree that doing a beta is a good idea. However, please let me do one more pass, to at least remove the "WIP:" prefixes from the commit messages. |
Sure @mthuurne, take your time :) |
This PR is only intended to share progress. It still needs work and once it is time to merge the changes should probably be split into smaller PRs for easier review.
Closes: #558