Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.2k
Support unsubstituted type variables with both a default and a bound or constraints#10789
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Support unsubstituted type variables with both a default and a bound or constraints#10789
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codspeed-hqbot commentedNov 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CodSpeed Performance ReportMerging#10789 willnot alter performanceComparing Summary
|
github-actionsbot commentedNov 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Coverage reportClick to see where and how coverage changed
This report was generated bypython-coverage-comment-action |
please review |
82ccf3b
toe519871
Compare7961f35
to183b6fa
Compare70991c2
to620d58a
CompareThanks for your contribution! We'll review this soon - working on pushing v2.10 out now, but I'm anticipating we can get this into v2.11 :) |
9840763
to58bea02
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Left some initial thoughts... I think maybe we could support an identical bound and default. From the original issue, this would require some addition like:
ifboundisnotNoneanddefaultisnotNoneandbound!=default:raiseRuntimeError(...)
Happy to get@Viicos' thoughts here too, he's got some great insight on detailed types like this.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I think it's safe to support all combinations with bounds, constraints and defaults. In any case, we prioritize the default type over T1=TypeVar('T1',bound=int,default=SubInt)-->generatesaschemafor`SubInt`T2=TypeVar('T2',int,str,default=int)-->generatesaschemafor`int` Having a different bound and default shouldn't be an issue. The only requirement -- from a static type checking perspective -- is that This should work: def_unsubstituted_typevar_schema(self,typevar:typing.TypeVar)->core_schema.CoreSchema:try:has_default=typevar.has_default()exceptAttributeError:# Happens if using `typing.TypeVar` on Python < 3.13passelse:ifhas_default:returnself.generate_schema(typevar.__default__)# pyright: ignore[reportAttributeAccessIssue]iftypevar.__constraints__:returnself._union_schema(typing.Union[typevar.__constraints__])iftypevar.__bound__:schema=self.generate_schema(typevar.__bound__)schema['serialization']=core_schema.wrap_serializer_function_ser_schema(lambdax,h:h(x),schema=core_schema.any_schema(), )returnschemareturncore_schema.any_schema() |
FyZzyss commentedNov 15, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Totally agree that it's safe. This code only works for unsubstituted models, so we simply prioritize the default type (expected behavior for me). I will take Viicos suggestion |
58bea02
tod03ad87
Compare4d46a2b
tocd4db43
Compare
It doesn't, only static type checkers enforce it :) |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
1a4c49e
tod6f8780
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This LGTM otherwise, just one minor suggestion :)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
db08e3e
toa3687bf
CompareCo-authored-by: Sydney Runkle <54324534+sydney-runkle@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks!
81b886c
intopydantic:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Change Summary
Add support for Bound and Default together in TypeVar:
Related issue number
fixes#9418
Checklist
Selected Reviewer:@sydney-runkle