-
Notifications
You must be signed in to change notification settings - Fork 26
Fix type annotations for xml_field_validator/serializer #277
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
Fix type annotations for xml_field_validator/serializer #277
Conversation
f34315c
to
6357e5b
Compare
@dapper91 the checks fail because PRs opened from forks don't get access to your codecov token, and so uploading of coverage information fails. You appear to be using a very old version of codecov action. If you upgrade to a newer one, this should work: https://docs.codecov.com/docs/codecov-tokens#commits-on-unprotected-branches |
6357e5b
to
8bd9725
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #277 +/- ##
=======================================
Coverage 92.30% 92.31%
=======================================
Files 30 30
Lines 1728 1730 +2
=======================================
+ Hits 1595 1597 +2
Misses 133 133
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@dapper91 I upgraded codecov action to fix the failure |
8bd9725
to
dd221f4
Compare
There are 2 issues with the annotations: 1. Function types are contravariant rather than covariant in their argument types. This means that `Callable[[BaseXmlModel], Any]` is a subtype rather than a supertype of `Callable[[MyModel], Any]` and so using `Callable[[BaseXmlModel], Any]` as a lower bound of TypeVars doesn't work. Since Python doesn't support upper bounds on TypeVars, the solution is to push the TypeVar inside. To see the issue run mypy on the example from the documentation[1]. Currently, you get these errors: ``` t.py:13: error: Value of type variable "ValidatorFuncT" of function cannot be "Callable[[type[Plot], XmlElementReader, str], list[float]]" [type-var] t.py:21: error: Value of type variable "SerializerFuncT" of function cannot be "Callable[[Plot, XmlElementWriter, list[float], str], None]" [type-var] ``` This PR resolves these (however, validate_space_separated_list needs to be decorated with @classmethod). 2. A recent commit[2] has a typo where ValidatorFuncT was bound by SerializerFunc instead of ValidatorFunc. [1] https://pydantic-xml.readthedocs.io/en/latest/pages/misc.html#custom-xml-serialization [2] dapper91@ec4b547
dd221f4
to
bc23aa1
Compare
There are 2 issues with the annotations:
Function types are contravariant rather than covariant in their argument types. This means that
Callable[[BaseXmlModel], Any]
is a subtype rather than a supertype ofCallable[[MyModel], Any]
and so usingCallable[[BaseXmlModel], Any]
as a lower bound of TypeVars doesn't work. Since Python doesn't support upper bounds on TypeVars, the solution is to push the TypeVar inside. To see the issue run mypy on the example from the documentation[1]. Currently, you get these errors:This PR resolves these (however, validate_space_separated_list needs to be decorated with
@classmethod
).A recent commit[2] has a typo where ValidatorFuncT was bound by SerializerFunc instead of ValidatorFunc.
[1] https://pydantic-xml.readthedocs.io/en/latest/pages/misc.html#custom-xml-serialization
[2] ec4b547