Do not inherit config in model fields that are themselves of type model

[dmontagu]

I think the current mechanism of attempting to merge configs when handling fields of model types is the source of some problems.

I'll describe the problems below, but given how we want to get the v2 release out ASAP, I really think we should drop this functionality in pydantic v2 (or at least find a way to make it possible to add in a future minor release) so that we don't find ourselves committed to dealing with bugs related to it.

I'll also note that this didn't seem to be a thing in v1 (a point which I'll make in the companion PR for this on the pydantic side), so it's not a breaking change to remove this behavior. (And in fact, improves backwards-compatibility.)

---

First, this is likely to be a source of bugs due to the fact that core refs don't reflect config settings, and if the schema gets simplified to use definitions, config settings will be lost. For example, consider:

from pydantic import BaseModel


class C(BaseModel):
    my_string: str


class B(BaseModel):
    model_config = dict(str_to_lower=True)

    c: C


class A(BaseModel):
    b: B
    c: C

If the schema is simplified in such a way that the C becomes a definition reference instead of duplicated, the behavior of C inside of B would be the same as inside of A. Right now we don't have a way to bake the config settings into the refs to prevent this, and I think it might be unreasonably difficult to try.

Even if we were smart about not out-of-lining C, I think you'll run into more challenging issues with recursive models. For example:

from pydantic import BaseModel

class B(BaseModel):
    model_config = dict(str_to_lower=True)

    a: 'A'

class A(BaseModel):
    b: B | None
    my_string: str

In this case, A has to be a definition reference, and to behave correctly where the string gets lowercased only for the cases where it is a subfield of B, you'd need to make sure to have different refs for the outer A and the one that has the config settings. One can also imagine more complex scenarios where the config takes different values at different levels of nesting; basically, if config influences the behavior of validation/serialization, it must be the same when the ref is the same. By eliminating config inheritance for fields, we eliminate the most common (and perhaps only?) way to get differing config with the same ref today.

---

Second, even if we did want to keep this functionality, it is currently causing problems because some config fields almost certainly should not get inherited by fields. The main example being extra, which is causing issue pydantic/pydantic#5784, which incidentally does get fixed by this PR.

---

In general, I would argue that if you want config inheritance, it's probably better to find a way to achieve the desired result by way of a shared base class, rather than trying to modify the config by way of inclusion in a different parent. I realize this is not equally capable, but I (currently) think its better to remove this capability and not try to support it for users in v2 (especially considering it wasn't even a thing in v1), than to deal with the various edge-case bugs it may cause.

The one exception to this would be if we made some kind of "global" config thing, so that it affected the entire core schema the same way, rather than being able to be overridden in fields. But I would argue that the current mechanism is too different to be a step toward that, and would prefer that we find a way to add that later.

[codspeed-hq]

CodSpeed Performance Report

Merging #612 no-inherit-config-in-model-fields (51be332) will not alter performances.

Summary

🔥 0 improvements
❌ 0 regressions
✅ 118 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

[codecov-commenter]

Codecov Report

Merging #612 (51be332) into main (2a7a2f6) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
- Coverage   94.25%   94.24%   -0.01%     
==========================================
  Files          98       98              
  Lines       13188    13158      -30     
  Branches       25       25              
==========================================
- Hits        12430    12401      -29     
+ Misses        752      751       -1     
  Partials        6        6              

Impacted Files	Coverage Δ
pydantic_core/core_schema.py	96.87% <ø> (-0.01%)	⬇️
src/build_tools.rs	86.25% <ø> (-1.80%)	⬇️
src/serializers/type_serializers/model.rs	93.96% <100.00%> (ø)
src/validators/model.rs	98.80% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

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

[adriangb]

This makes a lot of sense to me. The backward compatibility argument is a strong one. I think we should move forward with this as-is.

On the whole, I wonder what the value of CoreConfig here is in general: strict is a flag both on CoreConfig and also on every single schema, while str_to_lower is a flag on CoreConfig that only makes sense for str schemas. I think pydantic-core should do away with the config concept in general and pydantic should be the one that figures out how it wants to apply the config on a model (presumably using the model this PR is proposing of "pushing down" until the next config (or BaseModel) is encountered.

[samuelcolvin]

I think I agree.