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

Marshmallow ext: Name should be set to dump_to even if load_from is not specified #178

Closed
LeonAgmonNacht opened this issue Jan 9, 2018 · 2 comments

Comments

@LeonAgmonNacht
Copy link

commented Jan 9, 2018

At the moment, name is changed if both load_from and dump_to is set and are equal, as per this code:

def _observed_name(field, name):
    """Adjust field name to reflect `dump_to` and `load_from` attributes.
    :param Field field: A marshmallow field.
    :param str name: Field name
    :rtype: str
    """
    # use getattr in case we're running against older versions of marshmallow.
    dump_to = getattr(field, 'dump_to', None)
    load_from = getattr(field, 'load_from', None)
    if load_from != dump_to:
        return name
    return dump_to or name

This behaviour causes schema to look bad and does not make sense.
Name should be set to dump_to even if load_from is not specified.

@lafrech

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

Related to marshmallow-code/marshmallow#717.

I think this is a dark corner case.

I don't know how to expose a situation where a field is used for dump and load and the key differ in both dump and load. I'd use two schemas. Now, assuming the schema or the field is dump_only, it makes sense to only specify dump_to and expect it to appear in the spec. Conversely, if a schema is load_only, it should be exposed correctly by just specifying load_only.

AFAIK, OpenAPI does not provide the option for write_only fields, so dump_only might be the only real-life use case here in practice for now.

I suppose we could write

    # use getattr in case we're running against older versions of marshmallow.
    dump_to = getattr(field, 'dump_to', None)
    return dump_to or load_from or name

and cover most if not all legit cases. Right?

Yet, I don't really see a proper way to solve this and this is one of the motivations of marshmallow-code/marshmallow#717.

@lafrech

This comment has been minimized.

Copy link
Member

commented Jan 28, 2018

@sloria sloria closed this in 8b46251 Jan 31, 2018

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.