-
Notifications
You must be signed in to change notification settings - Fork 252
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
associated types for arguments in Input #1234
Conversation
Self: 'a; | ||
fn len(&self) -> usize; | ||
fn get_item<'k>(&self, key: &'k LookupKey) -> ValResult<Option<(&'k LookupPath, Self::Item<'_>)>>; | ||
fn iter(&self) -> impl Iterator<Item = ValResult<(Self::Key<'_>, Self::Item<'_>)>>; |
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.
It's the return-position-impl-trait-in-trait which requires Rust 1.76 for the MSRV bump. If we wanted to avoid that we probably can use associated types but it'll be gross by comparison (or we can Box
and suffer an allocation).
CodSpeed Performance ReportMerging #1234 will not alter performanceComparing Summary
|
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.
This looks really good, nice to get rid of some of those macros.
The last two big ones I think are in model_fields.rs
and typed_dict.rs
pos_value = $get_macro!(args, index); | ||
} | ||
// go through fields getting the value from args or kwargs and validating it | ||
for (index, field) in self.fields.iter().enumerate() { |
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.
it's pretty hard to review but I assume very little changed here.
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.
Yep if you hide whitespace from the diff it's a lot easier to see what changed.
Change Summary
Continuing with #1230
This time we move
GenericArguments
to have associated types.Related issue number
N/A
Checklist
pydantic-core
(except for expected changes)