Add support for deprecated fields
Change Summary
Starting implementing https://github.com/pydantic/pydantic/issues/2255
The new deprecated decorator could now be used as an annotation to mark fields as deprecated. The CPython implementation got tweaked to allow introspection by defining the decorator as a class instead as a function.
This PR is of course subject to change depending on how you want this to be implemented in Pydantic.
The tests basically show how it is possible to mark a field as being deprecated. A couple questions:
- Should we allow
deprecatedto be used as an argument toField/computed_field(as it is currently in this PR)? Or should we only allowAnnotated[..., deprecated(...)]/@deprecated(...)on a property? - If yes, should we allow
msgto be used here as well? (instead ofboolcurrently). - Should a runtime deprecation warning be raised when accessing a deprecated field? (This is already handled when used on a
computed_field)
Additional context: https://github.com/annotated-types/annotated-types/pull/34#issuecomment-1826859535
Related issue number
Checklist
- [ ] The pull request title is a good summary of the changes - it will be used in the changelog
- [ ] Unit tests for the changes exist
- [ ] Tests pass on CI
- [ ] Documentation reflects the changes where applicable
- [ ] My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers
Selected Reviewer: @sydney-runkle
CodSpeed Performance Report
Merging #8237 will not alter performance
Comparing Viicos:deprecated (4acb83a) with main (bbf0724)
Summary
✅ 10 untouched benchmarks
Please review (still in draft because I'm waiting for a stable release of typing extensions)
@Viicos,
Changes are looking good overall thus far. My initial thoughts:
Should we allow deprecated to be used as an argument to Field/computed_field (as it is currently in this PR)? Or should we only allow Annotated[..., deprecated(...)]/@deprecated(...) on a property?
Yes, we want to include deprecated as a parameter of Field
If yes, should we allow msg to be used here as well? (instead of bool currently).
The fewer new parameters we add to Field, the better, so I'm in favor of skipping the msg parameter in addition to the deprecated param. I would support, however, defining our own Deprecated dataclass similar to others in the types.py file. Then, we could do something like:
# either
some_field = Field(deprecated=Deprecated(True, msg='blah blah')
# or
some_field = Field(deprecated=True)
Reminds me a bit of the support we have for discriminator='some str' or discriminator=Discriminator(...) for Field.
Should a runtime deprecation warning be raised when accessing a deprecated field? (This is already handled when used on a computed_field)
I think yes, but let's see what @samuelcolvin thinks.
Additionally, we'll need to update both the API docs and the concepts docs with information about the various ways in which to use deprecated.
Depending on when 4.9.0 comes out for typing-extensions and our release timeline for 2.6.0, there's a possibility that we could include this as one of the big new feature releases in 2.6.0 🚀.
Isn't having Field(deprecated=Deprecated(True, msg="blabla")) too repetitive? I don't see a valid use case of having it set to False. My proposal here would be:
type deprecated as bool | str:
- if
False, no runtime warning is emitted, but"deprecated": falseis set explicitly on the JSON Schema. - if
True, it is set totruein the JSON Schema, and we either: emit no runtime warning or emit one with a default template message{field} is deprecated. - if
str, it is set totruein the JSON Schema, and we emit a runtime warning with the provided message.
If having str | bool isn't really suitable, we could still go with our own Deprecated dataclass but with msg as the sole argument?
Additionally, we'll need to update both the API docs and the concepts docs with information about the various ways in which to use
deprecated.
I'll probably add them once we get the implementation sorted out.
Would be nice to also support this on models:
from pydantic import BaseModel
from typing_extensions import deprecated
@deprecated("msg") # Either with the decorator or model_config
class Model(BaseModel):
pass
model_config = {'deprecated': True}
Model.model_json_schema()
#> {'properties': {}, 'title': 'Model', 'type': 'object', 'deprecated': True}
Probably better in another PR, but would be nice to have both features landing in the same version.
@Viicos,
That's fair, it's certainly redundant. some_field: X | Y = Field(discriminator=Discriminator(discriminator=some_func) feels a bit redundant too, but I think the case is worse for the Deprecated usage, when as you mentioned, we expect deprecated=True in most use cases.
I'm more in favor of a Deprecated dataclass, with just the msg arg as you've described. Though I see your point with the str | bool approach, I'd rather we separate the bool that's specifying the deprecated nature vs the msg used in the case of deprecated=True.
Let's wait and see what @dmontagu thinks about this API. Don't want to have you do more work than necessary.
Sounds good re docs 👍.
I agree, that'd be a nice feature on models. Indeed, a separate PR sounds like a good plan.
@Viicos if we are going to do this, I'd be inclined to try to match the API for warnings.deprecated proposed in PEP 702 as closely as possible. In particular, I would lean toward deprecated: str | None = _Unset as the annotation for this keyword argument, where None(or _Unset) means not deprecated, and if a string is passed, it should serve the same purpose, i.e., provide some deprecation message.
Is that reasonable to you?
I know the decorator from that PEP has some keyword arguments that affect the emitted warnings, but I think trying to support those (not to mention emitting runtime warnings when accessing the field) is outside the scope of this feature request/implementation right now. (If we want to consider doing that, I think it would merit a bigger design discussion.)
That said, I'll just note that I think we'll regret adding support for this if dataclasses adds support for marking a field as deprecated in some future PEP, and the approach used there is not API-compatible with this.
None(or_Unset) means not deprecated, and if a string is passed, it should serve the same purpose, i.e., provide some deprecation message.
Sounds good.
trying to support those [...] is outside the scope of this feature request/implementation right now. [...] I think it would merit a bigger design discussion.
Yes, I think this could be implemented later. I think for now I'll just implement emitting DeprecationWarning with a fixed stacklevel value (probably 2?). In the future, it would be nice to respect the arguments passed to deprecated, i.e.:
field: Annotated[int, warnings.deprecated("msg", category=MyCustomDepWarning, stacklevel=3)]
will emit a MyCustomDepWarning with stacklevel=3 when field is accessed. But this can be discussed later when actually implementing this functionality.
I'll just note that I think we'll regret adding support for this if
dataclassesadds support for marking a field as deprecated in some future PEP, and the approach used there is not API-compatible with this.
While I agree on this, no one has started a PEP related to this feature (or the hypothetical Deprecated type hint as discussed here and on following comments, although I don't think this solution will ever happen considering deprecated was moved to warnings to avoid confusion). So it might never happen after all, and it would be unfortunate to not have this deprecated feature in Pydantic just because we are waiting on a built in feature that might never happen.
Having it implemented in Pydantic could also be an inspiration to implement it in dataclasses (e.g. in the same way it is currently done here: via Field). The dataclass_transform spec could then also be aligned to support the deprecated argument in library field specifiers, making Pydantic compatible.
I didn't realize the typing_extensions.deprecated/warnings.deprecated was actually a class — I'm good with supporting an instance of that class as the argument to Field, so the kwarg annotation is deprecated: str | warnings.deprecated | typing_extensions.deprecated | None (can probably do some things to clean that annotation up but keep the same semantics).
The main question for me is whether it is worth allowing instances of this class to work as a standalone annotation to mark a field as deprecated, or if we should just require an annotation of Field(deprecated=deprecated(...)). I guess it's best to support adding deprecated(...) as an annotation, it's just annoying because the field version will work without any other changes, where I guess we'll have to do something specific to support a plain deprecated annotation. (I assumed you haven't added support for that yet but I didn't look closely..)
Nevermind, I see you added support for it as a plain annotation and it was basically trivial 👍
Nevermind, I see you added support for it as a plain annotation and it was basically trivial 👍
Yes thanks to Zac-HD who proposed a clever workaround ;)
Now using str for deprecated. I've mirrored the logic of the @deprecated decorator, that is it should emit a warning even if msg=''. Would it make sense to have empty strings set "deprecated": true on the JSON Schema but no warning is emitted?
~~Still need to emit the deprecation warning on attribute access. I tried defining a BaseModel.__getattribute__ I end up with recursion errors because I need to access model_fields/model_computed_fields to check if there is a deprecation message for the requested attribute. Plus __getattr__ makes use of object.__getattribute__ so this is tricky to handle. Any idea where I could raise such warnings? Wrapping deprecated model fields with a descriptor could do the trick, but there's probably a better way~~
To emit the deprecation warning, I had to set __pydantic_deprecated_messages__ at the class level. It is a mapping between deprecated field names and the corresponding deprecation message. An extra __getattribute__ is added on BaseModel which will look for a deprecation message and warn if required. Having this __pydantic_deprecated_messages__ at the class level is required to avoid infinite recursion with __getattribute__.
Please review
Had to update to latest pyright for deprecated to be recognized as a class. I will make a separate PR to fix new errors
@tiangolo sorry for the ping, I see there are some failures on the fastapi tests. How this feature is going to play with FastAPI? Will it be supported out of the box?
@Viicos,
Could you please review the conflicting files in this PR when you get a chance? Thanks a bunch!
I somehow totally missed/misread this comment, will apply changes soon
Docs probably require more work, maybe a dedicated section in concepts?
Docs probably require more work, maybe a dedicated section in concepts?
@Viicos, Yep, a concepts section would be great. Perhaps in the Concepts / Fields section?
Changes look good overall. Will do one last review once you've added docs :).
Working on getting a patch release out for 2.6 with some fixes, then will review this in more detail once that's out :).
I know the decorator from that PEP has some keyword arguments that affect the emitted warnings, but I think trying to support those (not to mention emitting runtime warnings when accessing the field) is outside the scope of this feature request/implementation right now. (If we want to consider doing that, I think it would merit a bigger design discussion.)
Thinking about this, I believe the implementation could be pretty straightforward. There can still be things to be discussed (i.e. is warnings.deprecated the only way to provide a category and stacklevel?), but I think I could get some time to work on it as part as the next release after this one gets merged.
If you still believe it requires more discussion, then we can delay a bit the implementation and could open an issue/discussion to see how this should be handled
As a proof of concept (no tests added), I pushed a commit that can support stacklevel and category from deprecated. It however requires typing_extensions>=4.9.0 (a DeprecatedFieldInfo class could be added otherwise, holding the msg/category/stacklevel attributes).
The new implementation seems to fix the performance issue, at least for non deprecated fields:
import time
import warnings
from pydantic import BaseModel, Field
class MyModel(BaseModel):
x: int = Field(deprecated='test')
y: int
my_model = MyModel(x=1, y=1)
t0 = time.time()
with warnings.catch_warnings():
warnings.simplefilter('ignore')
for _ in range(100_000):
my_model.x
# my_model.y
t1 = time.time()
print(t1 - t0)
With my_model.x: 0.05495929718017578
With my_model.y: 0.0045850276947021484
Test failure is probably because typing-extensions 4.10.0rc1 got released, parsing fails with release candidates. I'll fix it
Please review
I opened https://github.com/tiangolo/fastapi/issues/11182 to track FastAPI support
@Viicos,
Thanks! I'll do an in depth review this afternoon :).
To summarize things a bit:
- I reverted https://github.com/pydantic/pydantic/pull/8237/commits/cf06a7322d59aefc2e3ca607f0c5ee66d28f6757 (support for
categoryandstacklevel) for now, but can be added back - Once this one gets merged, ideally we could have support for deprecated models, and maybe Pydantic dataclasses?
- CI will fail with FastAPI until it is updated to handle this new feature. Once this PR is approved, I can find time to implement support in FastAPI
@Viicos,
Could you please move the pyproject.toml and package version parsing changes to a different PR, just so that we can keep this one entirely focused on the new deprecated field work?
Happy to review those changes as well, I just think having 2 PRs will make things a bit tidier.
- Reverted the
parse_mypy_versionchanges and reverted metadata changes (will open a new PR). - Replaced
from_deprecated_decoratorwith the proposed solution - I added some extra tests with validators and
validate_assignment.
One test will fail because a deprecation warning is emitted here:
@model_validator(mode='after')
def validate_x(self) -> Self:
self.x = self.x * 2
I'm wondering if we should use filterwarnings when calling validators internally? (I think it makes sense, users will want the deprecation warning to be emitted only when the field is accessed)
- [x] Reverted the parse_mypy_version changes and reverted metadata changes (will open a new PR).
Awesome!
- [x] Replaced from_deprecated_decorator with the proposed solution
Great! I've opened https://github.com/pydantic/pydantic/issues/8902 to address @dmontagu's request to have ComputedFieldInfo be explicitly non-public.
I added some extra tests with validators and validate_assignment.
Perfect!
I'm wondering if we should use filterwarnings when calling validators internally? (I think it makes sense, users will want the deprecation warning to be emitted only when the field is accessed)
Good question. I think it probably makes sense to put that burden on the user, but let me double check and get back to you in just a few minutes!
Given all of this, once we resolve the test question posed above ^^, I'll be ready to approve!
When we release 2.7 in a few weeks, we'll certainly highlight your spectacular work on this!! 🚀 Make sure to keep an eye out for that!
@Viicos,
Ah, and just one quick follow up question - were there changes to the parse_mypy_version function that you wanted to include in a different PR?
I'm wondering if we should use filterwarnings when calling validators internally? (I think it makes sense, users will want the deprecation warning to be emitted only when the field is accessed)
Good question. I think it probably makes sense to put that burden on the user, but let me double check and get back to you in just a few minutes!
Ah ok, yes let's leave this to the user. However, could you please add a section to the docs explaining how a user could filter these warnings if they desire to use a deprecated field in a model validator? Thanks a bunch!
Ah, and just one quick follow up question - were there changes to the
parse_mypy_versionfunction that you wanted to include in a different PR?
We initially wanted to make the function generic to parse other package versions, but turns out it doesn't work properly in some cases (with release candidates, but probably with a lot of features from PEP 440). That's why I reverted it to be specific for mypy, and if parsing package versions (and comparing them) is needed in tests, packaging should be used (as I did in this PR) as it will handle every edge case for us.
Ah ok, yes let's leave this to the user. However, could you please add a section to the docs explaining how a user could filter these warnings if they desire to use a deprecated field in a model validator? Thanks a bunch!
My reasoning for filtering warnings when calling validators was:
- You define your model with a property
a, that needs to be manually validated by assigning a new value:
class Model(BaseModel):
a: int = 2
@model_validator(mode="after")
def validate_model(self) -> Self:
self.a = self.a * 2
- Later on, you deprecated the
afield in favor ofa_new, with similar validation:
class Model(BaseModel):
a: Annotated[int, deprecated("use 'a_new'")] = 2
a_new: int = 5
@model_validator(mode="after")
def validate_model(self) -> Self:
self.a = self.a * 2
self.a_new = self.a_new * 2.5
- When instantiating
Model, I wouldn't want a deprecation warning to be emitted even if haven't provided any value fora, e.g.Model(a_new=3)[^1], especially as an "external" user of thisModelwho has no knowledge about the internal validation being performed. What do you think? (Of course, accessingModel(a_new=3).ashould still emit the warning).
[^1]: This makes me think: should a deprecation warning be raised when doing Model(a=...)?