[RFC] Don't pass attr, obj, data to fields (de)serialize methods
lafrech opened this issue · 4 comments
From a quick look, those are not used by existing fields. I suspect this might be or have been useful in specific fields such as Method
.
I can't investigate this thoroughly right now and anyway that would be a breaking change, but opening this here for discussion.
For separation of concerns, (de)ser methods shouldn't need the parent obj/data and field name.
Also, this means that one needs and object and a field name to serialize some data with a field, which may not always be the case. They may pass None
but then they don't respect the API (since it won't work with a field (de)ser method that actually relies on those). I faced this specific problem with EnumValue
field, when building self.choices
, see #2017.
I gave it a quick look.
attr
and obj
are needed in serialize
to get the value from the object.
In _serialize
, deserialize
and _deserialize
, they seem to be only used by Function
/ Method
fields, so removing those would allow us to simplify the implementation.
I guess some users rely on those fields. OTOH, if they didn't exist, I doubt we'd modify (de)ser methods to pass attr
and obj
just to add them.
I don't have a simple idea to replace those fields, though. pre_load/dump methods don't really answer because we don't want to modify the object. Unless we add some sort of proxy around the object but this sounds complicated.
An option could be to remove Function
and Method
fields and add an argument or two to Field
to allow the user to pass a custom function/method to get the value from the object. This function/method could be called from get_value
. In fact, it would be a get_value
override.
So it wouldn't be a field by itself but rather a customization of an existing field. There would be a benefit in terms of auto-documentation, since the type of the value would be known (the user may choose to use Field
for a type-free value).
We could add this feature and deprecate Function
and Method
(as well as the use of attr
and obj
) in marshmallow 3.x.
I was thinking of opening another issue about moving get_value
logic to the schema. The schema would just need to access the attribute
attribute of the field. If we do what I describe above, the schema would also need to call the custom accessor (the function/method). It's not obvious to me where the logic should be, but I still find it strange to pass an obj, an accessor function, etc.
We could add accessor
parameter to Field
.
class Field(FieldABC):
def __init__(..., accessor,...):
...
self.get_value = accessor
...
Allowing the user to do that:
def ser_function(...):
...
class MySchema(Schema):
def ser_method(...):
...
# Deprecated
old_function = fields.Function(ser_function)
# Replace with
new_function = fields.Field(accessor=ser_function)
# Deprecated
old_method = fields.Method(ser_method)
# Replace with
new_method = fields.Field(accessor=ser_method)
I still need to get my head around this. If Field
/Method
/Constant
used serialize
rather than _serialize
, we could remove the _CHECK_ATTRIBUTE
hack.
But then we wouldn't call get_value
so the simple solution above wouldn't work.
Also, it is worth noting that fields working at get_value
level couldn't be embedded in container fields, as those are meant to pass the value, not the object (according to this issue and #1995 (comment)). Perhaps we can live with this limitation. I don't see the point of using such a field in a container field.