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

Remove dump parameter from `schema2parameters` #114

Closed
sloria opened this issue Mar 6, 2017 · 7 comments

Comments

@sloria
Copy link
Member

commented Mar 6, 2017

As found by @lafrech in #112 : schema2parameters should only be handling client input, so dump should never be True. We should verify this and remove the dump parameter if possible.

@lafrech

This comment has been minimized.

Copy link
Member

commented Mar 6, 2017

Copying @frol who introduced dump parameter in https://github.com/marshmallow-code/apispec/pull/38/files.

I suspect frol introduced this parameter to let his change be a non-breaking enhancement. But maybe it should have been a bugfix instead, with dump parameter implicitly False.

@frol

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2017

Please, let me have a few hours to remember what was the point of this.

@frol

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2017

As you can see, I haven't introduced the dump parameter to fields2parameters, I only made the code consistent in this regard.

@jmcarp has introduced the dump parameter to fields2parameters in #33 to close issue #32.

@frol

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2017

I support the idea of dropping dump parameter for fields2parameters functions and pass dump=False explicitly to the downstream calls.

@lafrech

This comment has been minimized.

Copy link
Member

commented Mar 6, 2017

As you can see, I haven't introduced the dump parameter to fields2parameters, I only made the code consistent in this regard.

@jmcarp has introduced the dump parameter to fields2parameters in #33 to close issue #32.

Sorry. I read a little too fast.

In @jmcarp's flask-apispec, fields2parameters is called with dump=False as well.

I support the idea of dropping dump parameter for fields2parameters functions and pass dump=False explicitly to the downstream calls.

We may also drop dump parameter for field2parameter (singular) and call field2property with dump=False, can't we?

@frol

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2017

We may also drop dump parameter for field2parameter (singular) and call field2property with dump=False, can't we?

I believe, we should.

@sloria

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2017

Thanks for investigating this @lafrech and @frol.

Would either of you be willing to take a stab at a PR?

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