Implement deprecated field in json schema #9298
Conversation
CodSpeed Performance ReportMerging #9298 will not alter performanceComparing Summary
|
|
Please review |
|
@sydney-runkle I added a check for a truthy value in the |
pydantic/json_schema.py
Outdated
| return json_schema | ||
|
|
||
| @staticmethod | ||
| def _should_mark_class_as_deprecated(cls) -> bool: | ||
| return hasattr(cls, '__deprecated__') and getattr(cls, '__deprecated__') |
There was a problem hiding this comment.
getattr is enough but you can make it more explicit
There was a problem hiding this comment.
@PrettyWood Sure. Do you think bool(getattr(cls, '__deprecated__')) is fine?
There was a problem hiding this comment.
LGTM once we decide on the syntax for this line!
There was a problem hiding this comment.
I just made a remark regarding the code with hasattr + getattr.
I now took a bit of time to understand the context and reading https://peps.python.org/pep-0702/ it seems you only care about the presence of the field
I can decide to write
@deprecated("")
class A: ...
and I'll have A.__deprecated__ = "", which will be falsy and still valid.
So in the end I guess the initial approach with just return hasattr(cls, '__deprecated__') is the best
There was a problem hiding this comment.
What about a hasattr and then a false check? @NeevCohen @PrettyWood
There was a problem hiding this comment.
I guess that's reasonable, though @deprecated takes a str as a parameter, if anything else is passed in an exception is raised. The following doesn't work
from typing import deprecated
from pydantic import BaseModel
@deprecated(False)
class Model(BaseModel):
passRaises the following exception
TypeError: Expected an object of type str for 'message', not 'bool'I guess someone could manually set a __deprecated__ attribute on the class to False, though I'm not sure that's a use case as far as we're concerned.
I guess just the simple hasattr check is the best choice here.
@sydney-runkle @PrettyWood thoughts?
There was a problem hiding this comment.
+1. As I said earlier "So in the end I guess the initial approach with just return hasattr(cls, 'deprecated') is the best"
Change Summary
Reflect model depreciation in json schema
Related issue number
Related to #8922
Checklist
Selected Reviewer: @Kludex