Skip to content

Adds support for marshmallow @post_load #119

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jun 22, 2019

Conversation

elatomo
Copy link
Contributor

@elatomo elatomo commented Sep 24, 2018

Fixes #103

Marshmallow @post_load decorator can be used as an object factory, to directly return an instance instead of the usual dictionary.

This PR adds support for it, based on #104 plus tests.

Marshmallow `@post_load` decorator can be used as a factory object, to directly
return an instance instead of the usual dictionary.

This commit adds support for it.
@codecov-io
Copy link

codecov-io commented Sep 24, 2018

Codecov Report

Merging #119 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   97.64%   97.65%   +0.01%     
==========================================
  Files           8        8              
  Lines         339      341       +2     
==========================================
+ Hits          331      333       +2     
  Misses          8        8
Impacted Files Coverage Δ
flask_apispec/wrapper.py 98.11% <100%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 535fc9d...6ec36d3. Read the comment docs.

@tyarkoni
Copy link

tyarkoni commented May 4, 2019

Any chance this could be merged? It looks like a pretty common problem...

@marcus-oscarsson
Copy link

@jmcarp agree with @tyarkoni this seems like a common problem, what do you think about it ?

@rshah88
Copy link

rshah88 commented May 22, 2019

Can this be merged ? we are also hitting this.

@heckad
Copy link

heckad commented Jun 17, 2019

When it will be merged?

@rshah88
Copy link

rshah88 commented Jun 17, 2019

@sloria @jmcarp Can you guys help merge this ??

@sloria
Copy link
Collaborator

sloria commented Jun 18, 2019

Sorry for the delay on this. I can take a look and release this over the weekend.

@marcus-oscarsson
Copy link

@sloria Thanks !

@@ -43,8 +43,11 @@ def call_view(self, *args, **kwargs):
parsed = parser.parse(schema, locations=option['kwargs']['locations'])
if getattr(schema, 'many', False):
args += tuple(parsed)
else:
elif getattr(parsed, 'update', False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is risk of a false-positive here, e.g. model objects with an update method, so I think type-checking is preferable to duck-typing in this case.

I'll make this change myself.

@sloria sloria merged commit 98fdf21 into jmcarp:master Jun 22, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using use_kwargs with a schema which has @post_load
7 participants