New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate Field.include
#6852
Deprecate Field.include
#6852
Conversation
Deploying with Cloudflare Pages
|
please review |
docs/usage/serialization.md
Outdated
id: int = Field(..., include=True) | ||
username: str = Field(..., include=True) # overridden by explicit include | ||
id: int | ||
username: str # overridden by explicit include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this makes sense still?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I've just removed it.
@@ -793,6 +785,10 @@ def Field( # noqa: C901 | |||
if validation_alias in (_Unset, None): | |||
validation_alias = alias | |||
|
|||
include = extra.pop('include', None) # type: ignore | |||
if include is not None: | |||
warn('`include` is deprecated and does nothing. It will be removed, use `exclude` instead', DeprecationWarning) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIU, this is not a deprecation... If the functionality is removed, it's a removal. The parameter itself is deprecated.
Also, how is exclude
a replacement for include
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIU, this is not a deprecation... If the functionality is removed, it's a removal. The parameter itself is deprecated.
I think it is a deprecation as it did nothing before and still does nothing. So, no change in functionality. just warning the user.
Also, how is
exclude
a replacement forinclude
? 🤔
include=False
can be replaced with exclude=True
. That's why I suggested using exclude
.
42a0f88
to
510f961
Compare
Selected Reviewer: @Kludex