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

ModelSchema default behaviour needs a lot of overriding #36

Closed
paj28 opened this issue Oct 13, 2015 · 2 comments

Comments

@paj28
Copy link

commented Oct 13, 2015

The current behaviour of ModelSchema is not that useful to me. Where I have, say, an "order" many-to-one relationship, by default it dumps a field "order" with the order_id, and this serialised data does not then load. It would be more helpful to dump this as field "order_id". one-to-many relationships are similarly not that useful.

Right now I am using a lot of excludes and manually defined fields, but this is unnecessary. In fact, marshmallow-sqlalchemy already has much more helpful default logic, on TableSchema, especially with include_fk=True. I can't directly use TableSchema as it only supports dump and I also need to load.

However, a one line change to schema.py really helps:

return converter.fields_for_model(opts.model, fields=opts.fields, exclude=opts.exclude)
-->
return converter.fields_for_table(opts.model.__table__, include_fk=True, fields=opts.fields, exclude=opts.exclude)

I realise this would break compatibility, but it seems a much more helpful default to me.

@paj28

This comment has been minimized.

Copy link
Author

commented Oct 14, 2015

I've now realised I can do this quite easily with a custom converter:

class MyModelConverter(marshmallow_sqlalchemy.convert.ModelConverter):
    def fields_for_model(self, model, include_fk=True, fields=None, exclude=None):
        return self.fields_for_table(model.__table__, include_fk, fields, exclude)

jmcarp added a commit to jmcarp/marshmallow-sqlalchemy that referenced this issue Oct 25, 2015

jmcarp added a commit to jmcarp/marshmallow-sqlalchemy that referenced this issue Oct 25, 2015

@sloria sloria closed this in #39 Oct 26, 2015

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 26, 2015

@jmcarp Has just added the include_fk as a class Meta option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.