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

`many` argument to `Schema.jsonify()` should default to `Schema.many` #42

Merged
merged 4 commits into from Apr 30, 2016

Conversation

@singingwolfboy
Copy link
Contributor

commented Apr 29, 2016

I was very confused by the Schema.jsonify() API for collections -- I had to read through the code to understand what I was supposed to do. I thought that I only had to set Schema.many and everything would just work, but it turns out there's a many attribute to Schema.jsonify() that is handled completely separately. I figured it would make sense for jsonify() to know about Schema.many, so this pull request makes the change in a backwards-compatible manner.

I think that a better change would be removing the many argument from Schema.jsonify() entirely, and letting Schema.many control the mechanism entirely. However, I believe that removing that argument would break backwards compatibility, so I left it in for this pull request.

Note that the current released version of Flask does not allow passing a list to flask.jsonify(), and it will barf if you try. However, a recent pull request to Flask removes that restriction, so this functionality should work in Flask 1.0 -- or in bleeding-edge versions of Flask, if you swing that way. The new tests will be skipped if you use any version of Flask that is below 0.11 (bleeding edge).

singingwolfboy added some commits Apr 29, 2016

Saner default for `many` argument of `Schema.jsonify` method
When `Schema.many` is True, the `many` argument of the `Schema.jsonify`
method should also default to True.
@sloria

This comment has been minimized.

Copy link
Member

commented Apr 30, 2016

Looks like the correct behavior. Thanks!

@sloria sloria merged commit aef3d1a into marshmallow-code:dev Apr 30, 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.