Skip to content

Fix deprecation warnings regarding collections.abc #624

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

Merged
merged 4 commits into from
May 15, 2019
Merged

Fix deprecation warnings regarding collections.abc #624

merged 4 commits into from
May 15, 2019

Conversation

tirkarthi
Copy link
Contributor

@tirkarthi tirkarthi commented May 11, 2019

Fixes #613

Description of the Change

  • Fix DeprecationWarning over usage of collections instead of collections.abc

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

The PR doesn't seem to have user impacting changes. Let me know if these need to be updated.

Thanks.

@codecov
Copy link

codecov bot commented May 11, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   95.65%   95.66%   +0.01%     
==========================================
  Files          55       55              
  Lines        2856     2863       +7     
==========================================
+ Hits         2732     2739       +7     
  Misses        124      124
Impacted Files Coverage Δ
rest_framework_json_api/relations.py 85.5% <100%> (+0.21%) ⬆️
rest_framework_json_api/renderers.py 87.5% <100%> (+0.16%) ⬆️

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 4e4d55f...e74ba8c. Read the comment docs.

@codecov
Copy link

codecov bot commented May 11, 2019

Codecov Report

Merging #624 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   95.65%   95.66%   +<.01%     
==========================================
  Files          55       56       +1     
  Lines        2856     2861       +5     
==========================================
+ Hits         2732     2737       +5     
  Misses        124      124
Impacted Files Coverage Δ
rest_framework_json_api/compat.py 100% <100%> (ø)
rest_framework_json_api/relations.py 85.29% <100%> (ø) ⬆️
rest_framework_json_api/renderers.py 87.37% <100%> (+0.04%) ⬆️

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 4e4d55f...9e9885f. Read the comment docs.

@sliverc
Copy link
Member

sliverc commented May 14, 2019

Thanks for working on this.

I would prefer if we could move those imports into a compat.py as DRF does This way we will find the deprecation fixes again in the future in case we can delete it again.

I think a CHANGELOG entry will also be needed below Fixed as a deprecation warning fix is user facing e.g. for users who turn deprecation warnings into errors.

@tirkarthi
Copy link
Contributor Author

Currently there is no compat.py. Do you want me to create one and move my changes there?

@sliverc
Copy link
Member

sliverc commented May 15, 2019

yes best create one. This way we follow how DRF handles such issues.

@tirkarthi
Copy link
Contributor Author

I have modified my PR to use compat.py and also added a note on Changelog.

Copy link
Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@sliverc sliverc merged commit db5cd72 into django-json-api:master May 15, 2019
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.

Deprecation warning in renderers.py
2 participants