Include all annotated fields in order #715
Conversation
Codecov Report
@@ Coverage Diff @@
## master #715 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 2710 2807 +97
Branches 532 578 +46
=====================================
+ Hits 2710 2807 +97 |
|
please can you rebase and move the history change to |
dmontagu
force-pushed
the
field-ordering
branch
from
0dc8323 to
21dd6a5
Compare
|
as per #716 I guess we should implement |
pydantic/main.py
Outdated
| name=var_name, | ||
| value=value, | ||
| annotation=annotations.get(var_name), | ||
| class_validators=vg.get_validators(var_name), | ||
| config=config, | ||
| ) | ||
| new_fields_ordering = [k for k in annotations if k in new_fields] + [ |
There was a problem hiding this comment.
| new_fields_ordering = [k for k in annotations if k in new_fields] + [ | |
| ordered_new_fields = [k for k in annotations if k in new_fields] + [ |
pydantic/main.py
Outdated
| new_fields_ordering = [k for k in annotations if k in new_fields] + [ | ||
| k for k in new_fields if k not in annotations | ||
| ] | ||
| for k in new_fields_ordering: |
There was a problem hiding this comment.
| for k in new_fields_ordering: | |
| fields.update(ordered_new_fields) |
I think.
There was a problem hiding this comment.
new_fields_ordering only includes the keys, not the values; your modification above didn't change that, as far as I can tell. So ordered_new_fields is a List[str], not a Dict[str, Field].
I'm happy to rename, but I couldn't come up with a way to get ordered_new_fields to be a dict without making things ugly elsewhere. Let me know if you want me to try harder.
There was a problem hiding this comment.
Humm, that's true sorry.
Rather than describe it all, i'll commit to your PR and you can change or remove it if it doesn't make sense.
There was a problem hiding this comment.
take a look at d3b10e5, I think that's better than building the order wrong then rebuilding it right, but happy to change if I'm missing something.
dmontagu
force-pushed
the
field-ordering
branch
from
21dd6a5 to
62b543b
Compare
|
@dmontagu kind of unrelated, but nowhere better to discuss: Could I have your email address? If you're not happy sharing it publicly, feel free to email s@muelcolvin.com. |
Sent you an email, let me know if you didn't get it. |
|
@dmontagu are you happy with this? If so I guess it can be merged. |
|
@samuelcolvin I'm good with it, feel free to merge once the checks re-pass. |
Change Summary
Modifies field ordering so that all annotated fields occur in the order included.
Note: non-annotated fields still come last; this is probably unavoidable.
This is a subtly breaking change, so, if desired, should probably be part of 1.0.
Related issue number
Related to #674 #483 #203 #716
Checklist
HISTORY.rsthas been updated#<number>@<whomever>