Skip to content

Conversation

@samuelcolvin
Copy link
Member

@samuelcolvin samuelcolvin commented Oct 25, 2022

Feedback on this welcome, in particular @tiangolo and @PrettyWood.

Basically this is reverting the keys in error information to match pydantic V1.

While I'm in favour of "get it right" over "keep it compatible", I don't think the win here is worth the headache this this will cause users.

With this change, errors like like:

[
  {
    'type': 'foobar',  # new values but this is a marked of what type of error we've got, mostly for machines to read
    'loc': ('field_a', 'field_b'),  # loc remains a tuple, this gives a slight performance improve over using a list
    'msg': 'Whatever,  # message template with context substituted
    'input': '...',  # this is new, it's the value that was submitted to this field/item
    'ctx': {'foo': 123},  # not always included, the context used to render the msg
  },
  { ... },
  ...
]

Before this change (e.g. on main of pydantic-core atm.), errors looked like:

[
    {
        'kind': 'foobar',
        'loc': ['field_a', 'field_b'],
        'message': 'Whatever',
        'input_value': '...',
        'context': {'foo': 123},
    }
]

While this might be a bit better, I don't think it's enough better to make up for the hassle this change will cause people.

WDYT?

If we decide to change this, I'd rather do it in pydantic/pydantic#4516 than after to minimise the changes there.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2022

Codecov Report

Merging #307 (eda1058) into main (5731c23) will increase coverage by 0.00%.
The diff coverage is 98.37%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #307   +/-   ##
=======================================
  Coverage   97.36%   97.37%           
=======================================
  Files          55       55           
  Lines        6344     6352    +8     
  Branches       45       44    -1     
=======================================
+ Hits         6177     6185    +8     
  Misses        167      167           
Impacted Files Coverage Δ
src/errors/mod.rs 92.30% <ø> (ø)
src/validators/list.rs 100.00% <ø> (ø)
src/validators/time.rs 97.82% <ø> (ø)
src/errors/value_exception.rs 94.89% <93.10%> (+0.57%) ⬆️
src/input/input_json.rs 95.88% <93.75%> (ø)
pydantic_core/core_schema.py 100.00% <100.00%> (ø)
src/errors/line_error.rs 96.38% <100.00%> (ø)
src/errors/location.rs 98.18% <100.00%> (+0.06%) ⬆️
src/errors/types.rs 78.24% <100.00%> (ø)
src/errors/validation_exception.rs 98.18% <100.00%> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5731c23...eda1058. Read the comment docs.

@samuelcolvin
Copy link
Member Author

samuelcolvin commented Oct 25, 2022

Also reverted loc to be a tuple which allowed for some performance improvements, mostly because we can construct one empty tuple and use it in all the cases where loc is empty.

@tiangolo
Copy link
Collaborator

Makes sense, and as I was saying on DM, I don't have a personal preference, but maybe there's a way to gauge usage and how much would it break people's code. But it also makes sense to try and keep compatibility, if it's doable enough. 🤷

@PrettyWood
Copy link
Collaborator

PrettyWood commented Oct 26, 2022

I think it's expected for v2 to be a big change! People want to have the same features (and more) but already know it won't be free. I would go with whatever seems the best for the future (tuple looks good, naming too) even if it changes more compared to v1.
I feel like this is very easy to search and replace in a codebase. I expect people struggling way more with custom BaseModel with overridden dict or json methods or with code playing with internals

TLDR: I'm in favour of "get it right"

@samuelcolvin
Copy link
Member Author

TLDR: I'm in favour of "get it right"

Agreed. But on this occasion, I think what we have here is right.

I'll work on this tomorrow.

@samuelcolvin samuelcolvin merged commit 9693919 into main Oct 26, 2022
@samuelcolvin samuelcolvin deleted the error-changes branch October 26, 2022 22:22
davidhewitt pushed a commit to pydantic/pydantic that referenced this pull request Oct 20, 2025
* rename kind -> type

* renaming message -> msg

* rename context -> ctx

* rename input_value -> input

* make "loc" a tuple, not a list

Original-commit-hash: 9693919
davidhewitt pushed a commit to pydantic/pydantic that referenced this pull request Oct 22, 2025
* rename kind -> type

* renaming message -> msg

* rename context -> ctx

* rename input_value -> input

* make "loc" a tuple, not a list

Original-commit-link: pydantic/pydantic-core@9693919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants