Should marshmallow v5 set required names for hooks?
I've been thinking about #600 . One of the primary use-cases for work like #600 is to be able to subclass a schema and append or prepend transformations to post_load, pre_load, post_dump, and pre_dump.
So, here is my proposal for how to handle this:
marshmallow.decorators.set_hooktakes a string argument,hook_name- if the decorated method's name does not match
hook_name, that is an error
Then normal inheritance rules come into play. This means that you can call a superclass hook with super() and pass args.
Some additional ideas/consequences based on playing around with this:
- Hook names are decorator names with a suffixed
_h(for "hook") or_hookso thatpost_load(the decorator) andpost_load_hcannot conflict within a class. This is ugly, but avoids@post_loadchanging in meaning within a class body, which could be confusing. - Hook names include
manyso that e.g.post_load_handpost_load_many_hmay be written on the same schema class. - Field validation hooks are named by field, as in
validates_field_{fieldname}_h - If the base class hook sets
pass_original=False, a child hook can setpass_original=Trueand then discard original data in the super call, but ifpass_original=Truein the base class, child class hooks must also set it (seems like a fine constraint to me?)
Usage demo:
class A(ma.Schema):
foo = ma.fields.Str()
@ma.post_load
def post_load_h(self, data, **kwargs):
return {"bar": 1, **data}
class B(A):
@ma.post_load
def post_load_h(self, data, **kwargs):
d = super().post_load_h(data, **kwargs)
del d["foo"]
return d
class C(B):
@ma.post_load
def post_load_h(self, data, **kwargs):
data["save"] = dict(data)
return super().post_load_h(data, **kwargs)
print(A().load({"foo": "baz"})) # {'bar': 1, 'foo': 'baz'}
print(B().load({"foo": "baz"})) # {'bar': 1}
print(C().load({"foo": "baz"})) # {'bar': 1, 'save': {'foo': 'baz'}}Initial implementation is primarily:
--- a/src/marshmallow/decorators.py
+++ b/src/marshmallow/decorators.py
@@ -195,3 +218,7 @@ def post_load(
def set_hook(
- fn: Optional[Callable[..., Any]], key: Union[Tuple[str, bool], str], **kwargs: Any
+ fn: Optional[Callable[..., Any]],
+ *,
+ hook_name: str,
+ key: Union[Tuple[str, bool], str],
+ **kwargs: Any
) -> Callable[..., Any]:
@@ -209,3 +236,10 @@ def set_hook(
if fn is None:
- return functools.partial(set_hook, key=key, **kwargs)
+ return functools.partial(set_hook, hook_name=hook_name, key=key, **kwargs)
+
+ hook_name = f"{hook_name}_h"
+ if fn.__name__ != hook_name:
+ raise ValueError(
+ f"{hook_name} hook had incorrect name: {fn.__name__}. "
+ f"Rename it to {hook_name} to resolve."
+ )I'd like to know if this is a good path to pursue before trying to fixup the whole testsuite. The ValueError could be a warning in v3, and a lot of the desired value would be delivered immediately because it would push people to make their hook names consistent.
Another question is whether or not Schema itself should implement (stub/no-op) versions of these methods. That might be important for mixin schema use-cases.
Source: marshmallow-code/marshmallow