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

Method field #53

Closed
martinjuhasz opened this issue Sep 27, 2016 · 9 comments
Closed

Method field #53

martinjuhasz opened this issue Sep 27, 2016 · 9 comments

Comments

@martinjuhasz
Copy link

I'm in the need for computed attributes on my model objects.
Looking at marshmallow it seems the most apropriate way to do this is by implementing the Method field like:

class UserSchema(Schema):
    since_created = fields.Method("get_days_since_created")
    def get_days_since_created(self, obj):
        return dt.datetime.now().day - obj.created_at.day

Looking at umongo the method field is not implemented yet. For testing i tried subclassing a new Field-Type like

class MethodField(BaseField, marshmallow_fields.Method):
    pass

of course resulting in an error. Marshmallow seems not to have access to the method field of the subclass (at this position) since self.parent isn't the umongo object but the marshmallow schema object.

What would be the right path to implement the Method field funcionality? Was there already done something in this direction or should i dig deeper and find a solution on how to pass the method call to marshmallow?

@lafrech
Copy link
Collaborator

lafrech commented Sep 27, 2016

Hi.

AFAIK, nothing has been done already. @touilleMan opened an issue (#38) about it, meaning he does not need it right now but anyone willing to do it is welcome to send a PR.

I thought I'd need MethodField so I gave it a naive attempt uncommenting those fields in umongo code. I think I got the same error as you did and didn't try any further.

Right now, I have two use cases:

  • If I want the computed attribute in database, I want it typed (e.g., I want it to be an int, not just the result of a method, so I use an IntField). This is needed at least for automatic API documentation. I can't do that with MethodField. This is a limitation in Marshmallow's Method. This issue is related. Currently, what I do is create this attribute as a normal Field and use callbacks to update it upon commit. I would like to improve this by adding callbacks to get/set on any field so that the object is always up to date - I don't want to wait for object commit. The callback could be either on the computed attribute's get method (pretty much what Method does, except the Field would still have a type, so rather like what above issue is about) or on the set method of any field used for the computation (you could add a callback to Field_1's set method to recompute Field_2 based on Field_1 data, for instance). The choice would be a matter of performance, depending on how often those fields are modified and accessed.
  • If I don't want the attribute in database, I use a property. The shortcoming is that if I want to serialize that property through my API, I need to add it (and its type) explicitly to the Marshmallow schema as umongo doesn't know about it.

I hope I'm making sense...

I'm not sure MethodField would cover any of those use cases.

I didn't have the time yet to give a try to this setter/getter callback feature so I didn't post here about it.

@martinjuhasz
Copy link
Author

If I don't want the attribute in database, I use a property. The shortcoming is that if I want to serialize that property through my API, I need to add it (and its type) explicitly to the Marshmallow schema as umongo doesn't know about it.

This is pretty much the case i'm looking for. Since i'm using umongo objects and serialize them for my API i cannot find a way to achive this. Is the only way to redeclare my whole umongo object as a marshmallow schema and dump the umongo through it schema.dump(umongoobj)?

@lafrech
Copy link
Collaborator

lafrech commented Sep 27, 2016

You don't have to redeclare the whole schema from scratch. Export it from umongo, then adapt it to add the property.

See #34 and docs.

@martinjuhasz
Copy link
Author

ah ok, thanks for this! This will totally fit my needs. Since all these other technics exist i'm not sure for what to use the method/function functionallity of marshmallow in any way.

@lafrech
Copy link
Collaborator

lafrech commented Sep 30, 2016

I'm not sure either. Maybe @touilleMan has something in mind.

Should we close this issue as a duplicate of #38?

@martinjuhasz
Copy link
Author

yes, totally. thanks again!

@touilleMan
Copy link
Member

Hi,

I tried a bit to implement MethodField in umongo. As @lafrech stated it's not as simple as I thought at first !

  • MethodField doesn't give information about the type of object returned. So we cannot use it to do deserialization (i.e. loading data from user world).
  • MethodField called for serialization is meaningless for umongo : data stored in mongodb don't have to be recomputed when dumped to user
  • Marshmallow's Method field doesn't handle really well missing fields (for example if you want to compute an age from a datetime provided inside the payload to deserialize, you can't use a marshmallow.Method for the age because this field will be considered empty and so the deserialization method will never be called...)

So I'm thinking Method and Function fields are not fitted for umongo.
That's quite ok because umongo provides tight integration with marhmallow. Thus you can define&run a custom marshmallow Schema with Method field to do your advanced cooking before passing to the umongo Document.

Example:

@instance.register
class User(Document):
    birthday = DateTimeField(required=True)
    age = IntField(required=True)

class UserNoAgeSchema(User.schema.as_marshmallow_schema):
    class Meta:
        fields = ('birthday', )

def load_user(payload):
    ret = UserNoAgeSchema().load(payload)
    if ret.errors:
        # do something
    ret.data['age'] = (datetime.utcnow() - ret['birthday']).days / 365
    return User(**ret.data)

Or with the document hooks:

@instance.register
class User(Document):
    birthday = DateTimeField(required=True)
    age = IntField(required=True)

    def pre_insert(self):
        self._compute_age()

    def pre_update(self):
        self._compute_age()

    def _compute_age(self):
        self.age = (datetime.utcnow() - self.birthday).days / 365

@lafrech
Copy link
Collaborator

lafrech commented Sep 30, 2016

I still think it would be nice to have a way to call the callback at get or set time.

class User(Document):
    birthday = DateTimeField(required=True, on_set=self._compute_age)  # either here
    age = IntField(required=True, on_get=self._compute_age)  # or there

    def _compute_age(self):
        self.age = (datetime.utcnow() - self.birthday).days / 365

One nice thing with umongo is that validation occurs at set time, so the object is always valid. You don't have to wait until commit (or explicit validation) to check that. get/set callbacks would allow those dependencies between fields to be enforced anytime just as well.

@touilleMan
Copy link
Member

@lafrech this is an interesting feature, I should create an issue for it.

My 0.2$ on this: Given an umongo document keeps internally the data in mongo world representation, it would be a bit cumbersome to add a check to do dynamic evaluation when retrieving a field (like you propose with on_get)
On the other hand on_set seems much easier given data always goes through BaseField.deserialize_from_mongo method. We could add a simple check there to call the function if defined.
However this will create small troubles if multiple fields are updated and have their on_set param pointing on the same callback. User would like to have the callback called once with the final value, instead it will be called twice, first time with half the data up to date...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants