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

Avoid DeprecationWarning when importing OrderedDict #54

Merged
merged 2 commits into from
Feb 14, 2020

Conversation

kbevers
Copy link
Contributor

@kbevers kbevers commented Feb 12, 2020

in flask_restx/model.py and flask_restx/swagger.py logic is installed to
circumvent differences in Python2 and Python3 when importing from the
collections module. Some classes from collections has been moved to
collections.abc in Python3. OrderedDict is not one of them, hence an
ImportError is raised when trying to import OrderedDict from
collections.abc. This then leads to importing both OrderedDict AND
MutableMapping/Hashable from collections.abc which then raises a
DeprecationWarning since MutableMapping/Hashable lives in
collections.abc in Python3.

This patch should fix those DeprecationWarnings to be raised.

Output from running pytest in a project that uses flask_restx. Here the DeprecationWarnings are seen clearly. I've only fixed the ones pertaining to the Python standard library :

/Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/model.py:12
  /Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/model.py:12: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
    from collections import OrderedDict, MutableMapping

/Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/api.py:28
  /Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/api.py:28: DeprecationWarning: The import 'werkzeug.cached_property' is deprecated and will be removed in Werkzeug 1.0. Use 'from werkzeug.utils import cached_property' instead.
    from werkzeug import cached_property

/Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/swagger.py:12
  /Users/kevers/miniconda3/envs/webproj/lib/python3.7/site-packages/flask_restx/swagger.py:12: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from   1 # -*- coding: utf-8 -*-$
'collections.abc' is deprecated, and in 3.8 it will stop working
    from collections import OrderedDict, Hashable

  1 # -*- coding: utf-8 -*-$
-- Docs: https://docs.pytest.org/en/latest/warnings.html

SVilgelm
SVilgelm previously approved these changes Feb 12, 2020
@Colin-b
Copy link
Contributor

Colin-b commented Feb 12, 2020

Seems like a partial implementation of noirbizarre/flask-restplus#768

@kbevers
Copy link
Contributor Author

kbevers commented Feb 12, 2020

Seems like a partial implementation of noirbizarre/flask-restplus#768

You seem to be correct in that. Applying that patch here would be a better solution.

in flask_restx/model.py and flask_restx/swagger.py logic is installed to
circumvent differences in Python2 and Python3 when importing from the
collections module. Some classes from collections has been moved to
collections.abc in Python3. OrderedDict is not one of them, hence an
ImportError is raised when trying to import OrderedDict from
collections.abc. This then leads to importing both OrderedDict AND
MutableMapping/Hashable from collections.abc which then raises a
deprecation warning since MutableMapping/Hashable lives in
collections.abc in Python3.

This patch should fix those DeprecationWarnings to be raised.
OrderedDict is a member of the collections module and not
collections.abc. This goes for both Python2 and Python3.
@kbevers
Copy link
Contributor Author

kbevers commented Feb 13, 2020

I went and fixed the rest of the OrderedDict imports as well. Should pose a more complete fix to this.

@SVilgelm
Copy link
Contributor

@ziirish Do we need this or we are going to remove py27 support at all?

@kbevers
Copy link
Contributor Author

kbevers commented Feb 13, 2020

@ziirish Do we need this or we are going to remove py27 support at all?

This is an improvement irregardless of Python2 support. You can't import OrderedDict from collections.abc in either of Python2 or Python3. If you keep the status quo the code will continue to issue DeprecationWarnings. If you enforce Python3 only then you'll have to apply this fix anyway, although there will be other places in the code that also needs a bit of adjusting.

@SVilgelm
Copy link
Contributor

@kbevers thanks for clarification, you are right.

Copy link
Contributor

@SVilgelm SVilgelm 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 to me. Thank you

Copy link
Contributor

@ziirish ziirish left a comment

Choose a reason for hiding this comment

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

I'm okay with this change.

Thanks for your contribution!

@SVilgelm SVilgelm merged commit 3582c84 into python-restx:master Feb 14, 2020
@kbevers kbevers deleted the fix-ordereddict-import branch February 14, 2020 14:59
@kbevers kbevers mentioned this pull request May 13, 2020
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.

5 participants