CodSpeed Performance Report

Merging #1791 will not alter performance

_{Comparing NicolasPllr1:add-none-required-error-on-optional-fields (91b130b) with main (0cd11fe)}

Summary

✅ 163 untouched

please review

Thanks for the contribution.

Surely this is going to be a breaking change for lots of people?

I think if we were to accept this, it would need to be behind a config flag, and opt in.

Thanks for the quick reply!

Surely this is going to be a breaking change for lots of people?

I did not think of that ... If their program relies on the specific errors emitted / the number of errors, then yes it's a breaking change.

I think if we were to accept this, it would need to be behind a config flag, and opt in.

Happy to work on that if you think this makes sense / had value.

As per the versioning policy https://docs.pydantic.dev/latest/version-policy/#pydantic-v2 I think adding additional error detail is permitted without considering it breaking.

I'd be quite up for having this. Consider e.g. a three-member union:

from pydantic import BaseModel
class User(BaseModel):
 age: int | float | None
user = User(age='abc')

ValidationError: 2 validation errors for User
age.int
 Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='abc', input_type=str]
 For further information visit https://errors.pydantic.dev/2.12/v/int_parsing
age.float
 Input should be a valid number, unable to parse string as a number [type=float_parsing, input_value='abc', input_type=str]
 For further information visit https://errors.pydantic.dev/2.12/v/float_parsing

It feels like we should have an age.None member there!

I'd also be up to have this included without consider it breaking. Alternatively this could make it for V3, but I'm not sure introducing an opt-in mechanism under V2 is worth it (I don't think users will ever enable it, it's a welcomed improvement but not a necessity for end users).

Is there something I can/should do or is it now fully on your hands ?

Thanks anyways for the work and answers you gave!

I think we need to convince @samuelcolvin that this is fine to merge, and then we can ship it :)

We discussed this at length on our call today.

The decision is to go ahead with it, but test this feature against FastAPI and the list of other companies that we test with new versions of pydantic. If they all pass, or getting them to pass is trivial, we release it as a bug fix. If many of them fail or fixing them is non-trivial, we should defer this to v3.

I ran the third-party tests for this branch in pydantic/pydantic#12313, and got 11 failing tests (16 in reality, but 5 of them are from another change awaiting test fixes in FastAPI). Two other third party tests are failing but because they seem to be broken currently.

However, quite some Pydantic tests are broken due to this. Granted, we do a lot of assertions on errors, but considering the short timeline before 2.12, I'd say we either defer this to the next release or v3. @NicolasPllr1 I can make sure to have this PR conflict free over time.