-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Move deque schema gen to GenerateSchema class
#11239
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
Conversation
CodSpeed Performance ReportMerging #11239 will not alter performanceComparing Summary
|
Deploying pydantic-docs with
|
| Latest commit: |
34193ea
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f9fb4ec4.pydantic-docs.pages.dev |
| Branch Preview URL: | https://remove-deq-prep-anns.pydantic-docs.pages.dev |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
|
cc @adriangb, more progress on simplifying logic here :) No need for a review, but wanted to let you know. |
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.
Great
The fact that we can't access the metadata anymore to set the maxlen attribute shows a limitation of the current API, I'll maybe include it in my write up
I'm not opposed to allowing metadata access, though this opens a can of worms in terms of consistency with annotation application. |
Follow up to #10846
See the description of that PR for more detail.
One consequence here is slightly different behavior for
maxlenbeing set fordequetypes, but I think this is worth the significant simplification in internal logic we see here.max_lengthconstraints are still enforced, but we no longer monkeypatch themaxlenfrommax_lengthon the deque. If users want this, they should write a custom validator.I'll be writing an in depth explanation of changes in the changelog for v2.11 related to this PR sequence.