-
Notifications
You must be signed in to change notification settings - Fork 306
Add field name in serialization error #1799
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
Add field name in serialization error #1799
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1799 will not alter performanceComparing Summary
|
PydanticSerializationUnexpectedValue
error
PydanticSerializationUnexpectedValue
error
please review |
179bcc8
to
84a1843
Compare
I forced-push to rebase on main. There was a tiny conflict with the last commit ed0d1ca (with the |
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.
While we could definitely improve the path later, this is a good improvement for now -- thanks!
Fix matchers for enhanced serialization warnings due to pydantic/pydantic-core#1799.
Change Summary
The immediate user problem is warning/error messages not being helpful enough during serialization.
These messages currently only provide - in some cases - the input value and type information. Not the location.
So some users are having trouble debugging serialization problems as they don't know which model or fields are causing issues (see discussion #9317 or issue #10495).
This PR only adds the 'field name' information when available. And I tried to add it with minimal changes.
It felt natural to add this info in the list already giving the input value and type information (introduced in #1652 with a5f7af1).
However, there may be a bigger problem with error-handling for serialization. It's not as precise as what is done for validation. For example, no exact 'location' is given to the user in case of a warning/error. Here I only give the field name, not the full location.
In this sense, my changes feel like a 'hack' that does not solve the underlying problem. I think fixing the bigger problem is what #1454 is aiming for, so maybe it's fine only adding the field name like this for now.
Tests
I was surprised to find out that this change did not break any test!
So I extended one in
serializers/test_model.py
which was already testing warnings. Maybeserializers/root_model.py
could have one too.I am not sure I really understand why this was not tested in the first place, so I am not confident here.
Related issue number
Other relevant issues/discussions
How to trace Pydantic serializer warning without traceback pydantic#9317
Add field name/location in serialization warnings pydantic#10495
Standardize how we raise serialization warnings #1454
Clean up
PydanticUnexpectedValueError
#1652Examples
Original example from #1483
Original example (simplified)
Notice the new
field_name
entry in the list (the field is called 'int_value' in the original example, which may be confusing here ...)Nested models with a Union
This example is taken from a comment by Sydney in #1449.
Nested model with union
Notice that there is still some error duplication. The difference is that you get the 'field name' information in the warnings.
TypeAdapter with a dict (no change here)
TypeAdapter example from pydantic/pydantic#10495:
TypeAdapter example
Note that this PR does not change this warning.
As the user suggests, a more precise location would be useful here. Or just the 'key' to begin with. But this PR does not address that.
Checklist
pydantic-core
(except for expected changes)Selected Reviewer: @davidhewitt