-
-
Notifications
You must be signed in to change notification settings - Fork 627
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
Meta Decorator #2810
base: dev
Are you sure you want to change the base?
Meta Decorator #2810
Conversation
98a871c
to
73cbe0f
Compare
73cbe0f
to
a07a40a
Compare
def meta(*bases, **kwargs): ... | ||
|
||
|
||
def meta(*bases, **kwargs): |
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.
the @overload
usage seems a bit odd...does the approach with TypedDict
and total=False
not work? like what we do here?
marshmallow/src/marshmallow/fields.py
Lines 87 to 98 in f0c6afb
class _BaseFieldKwargs(typing.TypedDict, total=False): | |
load_default: typing.Any | |
dump_default: typing.Any | |
data_key: str | None | |
attribute: str | None | |
validate: types.Validator | typing.Iterable[types.Validator] | None | |
required: bool | |
allow_none: bool | None | |
load_only: bool | |
dump_only: bool | |
error_messages: dict[str, str] | None | |
metadata: typing.Mapping[str, typing.Any] | None |
or perhaps
def meta(
*bases: SchemaMeta,
fields: tuple[str, ...] | list[str] | None,
additional: tuple[str, ...] | list[str] | None,
# ...
**kwargs,
):
def wrapper(schema):
mro = bases if bases else (schema.Meta,)
meta = type(schema.Meta.__name__, mro, {
"fields": fields,
"additional": additional,
# ...
**kwargs
})
return type(schema.__name__, (schema,), {"Meta": meta})
return wrapper
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.
Unpack[TypedDict]
does not allow accepting extra kwargs, but we allow custom Meta
attributes.
Explicitly copying the args into a dictionary defines missing attributes as None
, which breaks inheritance.
One @overload
is all that is needed (and is valid), but mypy requires a sacrificial overload.
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.
Ah I see. In that case, maybe just add comments about why this approach is necessary. Could even include a link to that gh comment
|
||
@typing.overload | ||
def meta( | ||
*bases: SchemaMeta, |
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.
Should this be
*bases: SchemaMeta, | |
*bases: Schema.Meta, |
? Surprised mypy doesn't raise an error in the tests tho 🤔
Add an experimental decorator for declaring meta attributes that inherits by default.
Resolves #1879