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

Improved further #37: Schema instances introspection for `many=True` #53

Merged

Conversation

@frol
Copy link
Contributor

commented Feb 20, 2016

The reproduction example code:

from marshmallow import Schema, fields
from apispec.ext.marshmallow.swagger import schema2jsonschema

class A(Schema):
    q = fields.Integer()

print(schema2jsonschema(A(many=True)))

Current dev branch produces:

$ python bug_53.py
{'properties': {'q': {'format': 'int32', 'type': 'integer'}}}

After applying this PR:

$ python bug_53.py
{'type': 'array', 'items': {'properties': {'q': {'type': 'integer', 'format': 'int32'}}}}

BONUS: resolved fields global import and local arguments collision by importing marshmallow module instead of fields from marshmallow.

P.S. This issue is related to the recently PRed Bulk-type arguments support in webargs: marshmallow-code/webargs#91

Improved further #37: Schema instances introspection; (bonus - resolv…
…ed 'fields' global import and local arguments collision by importing marshmallow by itself)

frol added a commit to frol/flask-restplus-server-example that referenced this pull request Feb 21, 2016

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2016

CC @sloria

Could you please review this PR?

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 2, 2016

@frol Yes, I will get to this over the next few days. Thanks for the PR and for your patience!

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 6, 2016

This looks like a good change.

Would you mind adding a test for the new behavior?

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 6, 2016

Once this is tested, this is good to merge.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2016

@sloria I will probably have some time today to write the tests, otherwise, I will do them during next week.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2016

@sloria I have added the tests.

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 6, 2016

Thank you!

sloria added a commit that referenced this pull request Mar 6, 2016

Merge pull request #53 from frol/fix-schema-instances-introspection
Improved further #37: Schema instances introspection for `many=True`

@sloria sloria merged commit 730e7f4 into marshmallow-code:dev Mar 6, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.