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

Resolve marshmallow deprecation warning and support marshmallow 4.0 #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kreathon
Copy link

Proposal for #116.

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2022

Codecov Report

Merging #117 (0e4606c) into master (0556746) will decrease coverage by 0.14%.
The diff coverage is 84.61%.

❗ Current head 0e4606c differs from pull request most recent head 1790d39. Consider uploading reports for the commit 1790d39 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
- Coverage   98.44%   98.29%   -0.15%     
==========================================
  Files           7        7              
  Lines        1285     1291       +6     
==========================================
+ Hits         1265     1269       +4     
- Misses         20       22       +2     
Impacted Files Coverage Δ
flask_accepts/utils.py 97.87% <80.00%> (-2.13%) ⬇️
flask_accepts/utils_test.py 100.00% <100.00%> (ø)

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 0556746...1790d39. Read the comment docs.

@kreathon
Copy link
Author

I am not sure how to handle the test coverage decrease.

Proper testing could involve running multiple tox environments and merging the coverage reports.

@kreathon
Copy link
Author

Alternatively, marshmallow could be pinned to <4.0 for now.

@circulon
Copy link

@kreathon
did you get this resolved?
I would like to use flask_accepts with marshmallow 4 in my project

The issue looks to be here:

if marshmallow_version >= (3, 13, 0):
    _ma_key_for_fr_example_key = "dump_default"
    _ma_key_for_fr_default_key = "load_default"
else:
    _ma_key_for_fr_example_key = "default"
    _ma_key_for_fr_default_key = "missing"

where the lines after the else look to be ignored?
maybe something like this would work for the code coverage instead?

_ma_key_for_fr_example_key = "dump_default"
_ma_key_for_fr_default_key = "load_default"
if marshmallow_version < (3, 13, 0):
    _ma_key_for_fr_example_key = "default"
    _ma_key_for_fr_default_key = "missing"

I would be interested to know if this works?

@apryor6 can you provide any additional feedback?
Additionally how do we run the code coverage tests locally please?

@kreathon
Copy link
Author

kreathon commented Jul 26, 2022

@circulon, you can check the .circleci/config.yml to see how to run the code coverage tests locally.

But my fear is that this project is dead (and it is not about coverage)

@circulon
Copy link

But my fear is that this project is dead (and it is not about coverage)

Bugger ;(

@kreathon are you working with or know of any other projects that are active and provide similar functionality?

Cheers

@kreathon
Copy link
Author

@circulon I don't know. Personally, I am now sticking to plain Marshmallow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants