Deploying pydantic-docs with    Cloudflare Pages

View logs

CodSpeed Performance Report

Merging#10911 willnot alter performance

_{Comparingfix-type-checking-issue-model-fields (60522af) withmain (b985365)}

Summary

✅ 46 untouched benchmarks

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
main.py
Project Total

This report was generated bypython-coverage-comment-action

We are hitting undefined behavior between pyright and mypy here, when a property is defined both on the metaclass and the class itself. I think it's a bit unfortunate to havemodel_fields typed as a class variable, because it won't emit deprecation warnings here. Here is an alternative approach:

• Until V3, we use the following descriptor defined onBaseModel (not the metaclass):

Code sample inpyright playground

importwarningsfromtypingimportCallable,Generic,TypeVar,overloadfromtyping_extensionsimportdeprecatedfrompydantic.warningsimportPydanticDeprecatedSince211ModelT=TypeVar('ModelT',bound='BaseModel')R=TypeVar('R')classclassonlyproperty(Generic[ModelT,R]):def__init__(self,fget:Callable[[type[ModelT]],R],/)->None:self.fget=fget@overloaddef__get__(self,instance:None,owner:type[ModelT])->R: ...@overload@deprecated("Accessing this attribute on the instance is deprecated, and will be removed in Pydantic V3.",category=None,    )def__get__(self,instance:ModelT,owner:type[ModelT])->R: ...def__get__(self,instance:ModelT|None,owner:type[ModelT])->R:ifinstanceisnotNone:warnings.warn("Accessing this attribute on the instance is deprecated, and will be removed in Pydantic V3.",category=PydanticDeprecatedSince211,            )returnself.fget(owner)classBaseModel:@classonlyproperty@classmethoddefmodel_fields(cls)->dict[str,str]:        ...BaseModel.model_fieldsBaseModel().model_fields# type checker error

pyright will emit an error, and mypywill probablybe able to do so soon implements it in 1.14.

• In V3, we remove theclassonlyproperty helper and themodel_fields fromBaseModel to move it back to the model metaclass, as it is currently.

WDYT?

@Viicos,

Interesting idea. 2 reservations I have about this:

• The reason we included themodel_fields property onBaseModel in the first place was for backwards compatibility - folks sometimes accessmodel_fields on aBaseModel instance. Thus, I think decorating with theclassonlyproperty is a bit misleading.

I sort of poorly explained this in a comment:

# TODO: V3 - remove `model_fields` and `model_computed_fields` properties from the `BaseModel` class - they should only# be accessible on the model class, not on instances. We have these purely for backwards compatibility with Pydantic <v2.10.

• Class properties aren't supported by Python, so I think this might be doing a bit more type hacking / jumping through hoops than we really should be doing?

• Thus, I think decorating with theclassonlyproperty is a bit misleading.

The naming of the decorator can be improved, but with this approach, accessing the propertydoes work at runtime. However, it will emit a runtime deprecation warning and a type checker error/warning

• Class properties aren't supported by Python, so I think this might be doing a bit more type hacking / jumping through hoops than we really should be doing?

Well the usage of the descriptor can feel a bit too much for such a small use case, but descriptor support is implemented by all major type checkers and this seems to be the only viable approach to emit a deprecation warning. I'm a bit worried that in V3 this will come as a surprise, because we currently don't emit anything (as an example, SQLModelrelies on this)

Yeah, fair point.

What if:

• For v2.10, we go with the approach on this PR
• For v2.11, we go with your approach (I don't want to introduce a deprecation warning in a patch release 😓)

Sounds good, could you please add some tests in thetypechecking folder? Probably inbase_model.py.

Oh so odd, I think I have them locally but didn't push...