Skip to content
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

fields.Method result should be able to be wrapped into other field types #513

Closed
gukoff opened this issue Aug 18, 2016 · 12 comments
Closed

Comments

@gukoff
Copy link

gukoff commented Aug 18, 2016

fields.Method is pretty much always used to return a certain type of value. It doesn't allow to choose options for the return value though.

For example, if one designes a field which should return a generic datetime and format it using a given format string, one is forced to use fields.Method which is linked to a schema method which returns a string(!), not a datetime. Because it is now impossible to provide a format string for the datetime anywhere except of the method itself.

In other words, something like this:

class MySchema(Schema):
    created_at = fields.Datetime(method='_created_at', format='%Y-%m-%dT%H:%M:%S%z')

    def _created_at(self, obj):
        return obj.metadata().creation_time()

would be much more convenient and reusable than this:

class MySchema(Schema):
    created_at = fields.Method('_formatted_created_at')

    def _formatted_created_at(self, obj):
        return obj.metadata().created_at().strftime('%Y-%m-%dT%H:%M:%S%z')
@maximkulkin
Copy link
Contributor

The Method and Function fields are actually non-beloved offsprings here as they do not stand for a particular type but change the way you access data. Other field types actually expect data to be accessed via properties.
A bit of self promotion to not leave you with no answers: I'm writing a library similar to Marshmallow that has this part done right. Check it out at https://github.com/maximkulkin/lollipop

@sloria
Copy link
Member

sloria commented Mar 19, 2017

This is simple enough to implement as a custom field.

from marshmallow import fields

class TypedMethod(fields.Method):

    def __init__(self, inner_field, *args, **kwargs):
        self.inner_field = inner_field
        super().__init__(*args, **kwargs)

    def _deserialize(self, value, attr, data):
        preprocessed = super()._deserialize(value, attr, data)
        return self.inner_field.deserialize(preprocessed)

    def _serialize(self, value, attr, obj):
        preprocessed = super()._serialize(value, attr, obj)
        return self.inner_field._serialize(preprocessed, attr, obj)

Usage:

import datetime as dt
from marshmallow import Schema, fields

class MySchema(Schema):
    created_at = TypedMethod(fields.DateTime('%Y-%m-%dT%H:%M:%S%z'), serialize='_created_at')

    def _created_at(self, obj):
        return obj.creation_time


class Thing:
    def __init__(self):
        self.creation_time = dt.datetime.utcnow()

schema = MySchema()
obj = Thing()
data = schema.dump(obj).data
assert isinstance(data['created_at'], str)
assert data['created_at'] == obj.creation_time.strftime('%Y-%m-%dT%H:%M:%S%z')

Closing this for now. I'm going to hold off on adding this to core, as the custom field above is a simple workaround. We can reopen if there is further interest in adding this to core.

@sloria sloria closed this as completed Mar 19, 2017
@lig
Copy link

lig commented Oct 12, 2018

@sloria I think the main problem here that the field's schema and the data being returned on serialization are mixed up.

So, thinking from the architectural point of view first we need to provide a schema, such as fields.DateTime and then we would like to get the data for the field from an unusual source, say other field or a method. It could look like this

class MySchema(Schema):
    created_at = fields.DateTime('%Y-%m-%dT%H:%M:%S%z', from_field='creation_time')

or for a more complex cases

class MySchema(Schema):
    created_at = fields.DateTime('%Y-%m-%dT%H:%M:%S%z', from_method='get_created_at')
    def get_created_at(self, obj):
        return obj.creation_time or datetime.now()

In such a case fields.Method becomes redundant or at least rarely needed while keeping any plugin happy knowing the exact schema.

@lafrech
Copy link
Member

lafrech commented Oct 12, 2018

from_field already exists. It is the attribute parameter.

We could add parameters dump_method and load_method. And deprecate Method field.

And likewise *_function, unless there is a way to do both functions and methods with the same parameter.

@lig
Copy link

lig commented Oct 12, 2018

@lafrech It would be great!

@lafrech
Copy link
Member

lafrech commented Oct 12, 2018

@lig you may open an issue for this, to see how the idea is received, and eventually work on a PR.

@lig
Copy link

lig commented Oct 12, 2018

@lafrech I believe, this issue is exactly about this

@lafrech
Copy link
Member

lafrech commented Oct 12, 2018

Alright. Let's reopen this.

@lafrech lafrech reopened this Oct 12, 2018
@richard-to
Copy link

richard-to commented Dec 26, 2018

Yes, this would be a nicer feature to have. My current workaround is to create one attribute for serialization and one for deserialization.

Example:

# Used during Deserialization
foo = Nested(
    String,
    load_only=True,
)

# Used during Serialization
_foo = Method(
    'serialize_foo',
    data_key='foo',
    dump_only=True,
)

@kettenbach-it
Copy link

I like this too. It would solve this: #1638

@1Mark
Copy link

1Mark commented Aug 9, 2022

Any progress on this?

@lafrech
Copy link
Member

lafrech commented Jan 12, 2025

This change is being discussed in #2039. In fact in #2039 (comment), I shared a revelation I just had, which happens to be pretty close to what I had written here in #513 (comment) six years ago.

I may be a bit slow, but at least I'm consistent.

@lafrech lafrech closed this as completed Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants