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

Subclassed Lists marked as string type #433

Closed
andrjohn opened this issue Apr 16, 2019 · 3 comments

Comments

@andrjohn
Copy link
Collaborator

commented Apr 16, 2019

I am using a tool to generate a Marshmallow schema that I then present as a RESTful API. In order to override some of the methods of marshmallow.fields.List, the generated schema can have generated classes that subclass List. The resulting APISpec is almost perfect because openapi.py mostly uses isinstance to see if the field is a list. For example:

    is_array = isinstance(
        field, (marshmallow.fields.Nested, marshmallow.fields.List)
    )

However, when working out the field type the mapping is looked up using a concrete type:

    self.field_mapping.get(type(field), ("string", None))

It isn't straightforward for me to add the new types using map_to_openapi_type because of the generated nature of my marshmallow and the way the presentation layer is kept separate. However, instead of falling back to ("string", None), we can either: fall back to ("array", None) for instances of a List (and Nested?) either by explicitly checking for those types in field2type_and_format; or walking through the field_mapping (in order), checking for instances of each type.

@andrjohn andrjohn changed the title Subclassed Lists are result in string types Subclasses Lists marked as string type Apr 18, 2019

@andrjohn andrjohn changed the title Subclasses Lists marked as string type Subclassed Lists marked as string type Apr 18, 2019

@andrjohn

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2019

Thinking about this further, I think the correct thing to do is improve the behaviour of field2type_and_format when the type isn't explicitly found in the field_mapping. It currently always falls back string, but we can identify Lists as array and Dicts as object. In fact the DEFAULT_FIELD_MAPPING could be used, although not in its current form.

Note, things seem to work for subclassed Nested fields because they get fixed up after the fact in field2property. IMO, the type specific fix ups could be combined with field2type_and_format to put all the class/type specific properties in one place.

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

@andrjohn Would you be up for sending a PR?

@andrjohn

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2019

Done. Looking through the open issues I see someone had a problem with Integers a while ago (#250), although resolved it with @ma_plugin.map_to_openapi_type. I have added a test for this case as this method fixes that too.

@sloria sloria closed this in #435 Apr 24, 2019

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.