fix coverage and make typing-extensions a required dependency #2368
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2368 +/- ##
===========================================
+ Coverage 99.97% 100.00% +0.02%
===========================================
Files 23 23
Lines 4500 4485 -15
Branches 909 909
===========================================
- Hits 4499 4485 -14
+ Misses 1 0 -1
Continue to review full report at Codecov.
|
|
Do you think it's worth changing to depending on I'm unsure if anyone's raised it, but could also prevent issues with type checkers who don't use |
| @@ -7,8 +7,9 @@ | |||
| from pydantic.fields import Undefined | |||
| from pydantic.typing import Annotated | |||
|
|
|||
| pytestmark = pytest.mark.skipif(not Annotated, reason='typing_extensions not installed') | |||
|
This is a change to the
I think that's a good idea @JacobHayes suggested it on the original PR, I'll see how easy this would be now. |
There was a problem hiding this comment.
PR LGTM! The world is probably a better place without _FalseMeta 😁
Do you think it's worth changing to depending on typing-extensions>=3.7.4.3 going forward?
That should remove the need for all the custom implementations of get_args, and guarantees availability of Annotated.
It's probably a net benefit, but there were a few caveats to making things very clean:
I was able to refactor the get_args/get_origin stuff a bit with typing_extensions, but it's not a panacea:
- those helper funcs aren't available on 3.6 so I kept the old "polyfills"
- I had to remove two tests using a bare Callable (the typing_extensions functions seem to expect the Callable generics to be [...]). Might get just as messy again to add the shim if those are truly valid cases (creating a Model with a bare Callable field hint seemed to still work)
- Looks like this makes the "test compiled without deps" fail (since we now import typing_extensions). Does this need to be tweaked somehow?
It might be pretty tidy after dropping py 3.6 though, I think(?) the other two are more minor. Looks like @samuelcolvin whipped that up pretty quick. 👍
samuelcolvin
force-pushed
the
fix-coverage-2
branch
from
0bd4422
to
281450b
Compare
samuelcolvin
changed the title
|
Looks ready. @PrettyWood are you happy with this? I ❤️ negative PRs 😍. |
| 'Fields will therefore be considered all required or all optional depending on class totality.', | ||
| UserWarning, | ||
| if not hasattr(typeddict_cls, '__required_keys__'): | ||
| raise TypeError( |
There was a problem hiding this comment.
This change should probably be explicitly noted as a breaking change in the changelog, just in case there are people ignoring the warning.
I can also think of scenarios where people are using another libraries TypedDict, but I'm not sure if that's common or not (I've never done this before).
There was a problem hiding this comment.
No need it's in the same new version v1.8 ;)
|
@samuelcolvin We could maybe update |
pydantic/typing.py
Outdated
|
|
||
| # Annotated[...] is implemented by returning an instance of one of these classes, depending on | ||
| # python/typing_extensions version. | ||
| AnnotatedTypeNames = {'AnnotatedMeta', '_AnnotatedAlias'} |
There was a problem hiding this comment.
Should this be moved up since it's referenced in the branches above?
|
fix #2367