Add support for deprecated fields

[Viicos]

Change Summary

Starting implementing #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 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?
• If yes, should we allow msg to be used here as well? (instead of bool currently).
• Should a runtime deprecation warning be raised when accessing a deprecated field? (This is already handled when used on a computed_field)

Additional context: annotated-types/annotated-types#34 (comment)

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-hq]

CodSpeed Performance Report

Merging #8237 will not alter performance

_{Comparing Viicos:deprecated (4acb83a) with main (bbf0724)}

Summary

✅ 10 untouched benchmarks

[Viicos]

Please review (still in draft because I'm waiting for a stable release of typing extensions)

[sydney-runkle]

@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 🚀.

[Viicos]

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": false is set explicitly on the JSON Schema.
• if True, it is set to true in 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 to true in 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.

[sydney-runkle]

@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.

[dmontagu]

@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.

[Viicos]

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 dataclasses adds 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.

[dmontagu]

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..)

[dmontagu]

Nevermind, I see you added support for it as a plain annotation and it was basically trivial 👍

[Viicos]

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 ;)

[dmontagu]

I think we should not filter deprecation warnings when calling validators.

E.g., consider the case where a @model_validator is defined on a user-defined subclass of a library-provided type:

class LibraryModel(BaseModel):
    a: Annotated[int, deprecated("use 'a_new'")] = 2
    a_new: int = 5

class UserModel(LibraryModel):
    @model_validator(mode="after")
    def validate_model(self) -> Self:
        self.a = self.a * 2

In this case I think it would be rather unfortunate if the user didn't see a deprecation warning (and failed to update their logic as appropriate!) because it was suppressed, thinking the user knew the field was deprecated.

Even if I am just only working in a single package, it still feels problematic to me that I don't get notified about my use of a deprecated field just because Pydantic thought I knew what I was doing and was accessing it on purpose. (I may well not have been.)

Even if there is some way to handle it that we think works slightly better in some cases, it sounds to me like it would require a decent amount of complexity, could still have surprising edge cases, and I just don't think that asking users to do the following is that high a price:

import warnings
...
    @model_validator(mode="after")
    def validate_model(self) -> Self:
		with warnings.catch_warnings():
		    warnings.simplefilter("ignore", DeprecationWarning)
			self.a = self.a * 2
        self.a_new = self.a_new * 2.5

It just seems to me that if you have the ability to mark a field as deprecated, surely you can either modify the validator as above, or it's in someone else's code where they should be made aware the field is deprecated. (And again, if you can modify the validator, it seems just as likely to me that, in a big enough codebase, you might miss that the field is deprecated and that you should.)

[dmontagu]

This makes me think: should a deprecation warning be raised when doing Model(a=...)

I think that, assuming this PR isn't already doing that, let's leave that (and any accompanying debate) for a separate PR and just get this merged.

[Viicos]

In this case I think it would be rather unfortunate if the user didn't see a deprecation warning (and failed to update their logic as appropriate!) because it was suppressed, thinking the user knew the field was deprecated.

This is true, and considering we can't really know if the model is user-defined (and in that case we should raise the warning) or provided by a library, having the warning emitted seems to be the best option (and as you said a filter can still be added in the model validator!)

[sydney-runkle]

@Viicos - last thing - could you please add a note to the docs suggesting how a user might filter / catch the warning, if desired?

Other than that, looks great!

[Viicos]

could you please add a note to the docs suggesting how a user might filter / catch the warning, if desired?

Yes forgot about this.

How should we ago about the fastapi issues? Should we disable the CI check for now while I work on it?

[sydney-runkle]

Truly amazing work!! 🚀

[sydney-runkle]

@Viicos,

Re the FastAPI issues - why are the tests failing? Do you expect the fix to be relatively minor? If so, we could use xfails on those tests, but I want to better understand the reasoning behind the failed tests before we merge!

[Viicos]

You will find more context in the FastAPI issue, but basically FastAPI uses a Param subclass of FieldInfo and adds a boolean deprecated attribute.

Do you expect the fix to be relatively minor?

I'm not sure how it will be handled yet, but I don't expect it to be too much work. Will also depend on how it will get reviewed.

[sydney-runkle]

@Viicos,

Ah ok, thanks for linking the issue with more clarification. I don't think this is a case where we can just skip the FastAPI tests.

I'm going to take a quick look + see if we can make a few minor changes on the FastAPI repo to guarantee that the existing behavior there is still preserved, while also supporting this new field on the pydantic end. I'll keep you updated with progress.

[sydney-runkle]

Ah ok so at a first glance, this fixes the fastapi tests:

diff --git a/fastapi/params.py b/fastapi/params.py
index b40944db..bf998d75 100644
--- a/fastapi/params.py
+++ b/fastapi/params.py
@@ -115,6 +115,7 @@ class Param(FieldInfo):
                     "serialization_alias": serialization_alias,
                     "strict": strict,
                     "json_schema_extra": current_json_schema_extra,
+                    "deprecated": deprecated,
                 }
             )
             kwargs["pattern"] = pattern or regex
@@ -568,6 +569,7 @@ class Body(FieldInfo):
                     "serialization_alias": serialization_alias,
                     "strict": strict,
                     "json_schema_extra": current_json_schema_extra,
+                    "deprecated": deprecated,
                 }
             )
             kwargs["pattern"] = pattern or regex

I could very well be overlooking something here, but if the change is this minimal, I'm feeling optimistic.

[Viicos]

Thanks for taking a look a it. My concern is that FastAPI currently defines deprecated as a bool (while we defined it as str | deprecated on our base FieldInfo class):

        deprecated: Optional[bool] = None,
        include_in_schema: bool = True,
        json_schema_extra: Union[Dict[str, Any], None] = None,
        **extra: Any,
    ):
        self.deprecated = deprecated

As FastAPI doesn't support str or deprecated instances (and I don't know if it should, it is only relevant for the JSON/OAS Schema), perhaps current_json_schema_extra could be updated to have "deprecated": True being set?

[sydney-runkle]

perhaps current_json_schema_extra could be updated to have "deprecated": True being set

Yeah, this seems like a better approach, given the type mismatch.

[sydney-runkle]

Let's see what @dmontagu thinks, as well.

[sydney-runkle]

Alright, so the conclusion was that even though this goes against our original thinking here, the easiest thing would be to support bool type deprecated specifications. This doesn't require any changes on the FastAPI end for basic compatibility (so we can pass all tests), and the diff is relatively small for this change.

I'll push this change shortly, along with a few tests, and flipping the fastapi tests check back on!