Skip to content

Address collections.abc import warning introduced in Python 3.7 - #124

Merged
zejn merged 2 commits into
mpdavis:masterfrom
peteryin21:import_from_collection_abc
Apr 9, 2019
Merged

Address collections.abc import warning introduced in Python 3.7#124
zejn merged 2 commits into
mpdavis:masterfrom
peteryin21:import_from_collection_abc

Conversation

@peteryin21

@peteryin21peteryin21 commented Jan 15, 2019

Copy link
Copy Markdown
Contributor

Description

In Python 3.7, I am seeing the following warnings from this library:

 /usr/local/lib/python3.7/site-packages/jose/jwt.py:5: 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 Mapping
/usr/local/lib/python3.7/site-packages/jose/jws.py:6: 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 Mapping, Iterable

This warning was introduced in this CPython commit.

To ensure future compatibility with Python 3.8, I've updated these imports to try importing from collections.abc and fall back to collections to keep backward compatibility with Python 2.

In addition, I made a few updates to the requirements-dev.txt that I needed to run the tests in Python 3.7.

  • Added conditional to only install wsgiref in Python 2, since it's not supported in Python 3 (and is instead included in the standard library)
  • Removed ecdsa since it is a duplicate requirement from requirements.txt and pip complains
Double requirement given: ecdsa (from -r requirements.txt (line 5)) (already in ecdsa==0.13 (from -r requirements-dev.txt (line 10)), name='ecdsa')
  • Updated pytest, py, pytest-cov, coverage, and coveralls packages to get tests to pass with Python 2 and 3.
  • Added cryptography package required for tests.

Testing

I created a virtualenv with both Python 2.7.14 and Python 3.7.1, ran pip install -r requirements-dev.txt and then ran the tests with pytest tests. Output below

Python 2.7.14

(venv2) ➜ python-jose git:(master) ✗ pytest tests
================================================================================================================================= test session starts =================================================================================================================================
platform darwin -- Python 2.7.14, pytest-4.1.1, py-1.5.4, pluggy-0.8.1
rootdir: /Users/peteryin/code/open_source/python-jose, inifile: setup.cfg
plugins: cov-2.6.1
collected 184 items
tests/test_firebase.py ... [ 1%]
tests/test_jwk.py ......... [ 6%]
tests/test_jws.py ...................................... [ 27%]
tests/test_jwt.py ................................................. [ 53%]
tests/test_utils.py .. [ 54%]
tests/algorithms/test_EC.py ......................... [ 68%]
tests/algorithms/test_HMAC.py ... [ 70%]
tests/algorithms/test_RSA.py .................................................. [ 97%]
tests/algorithms/test_base.py .. [ 98%]
tests/rfc/test_rfc7520.py ... [100%]
================================================================================================================================== warnings summary ===================================================================================================================================
tests/test_jwt.py::TestJWT::test_invalid_claims_json
/Users/peteryin/code/open_source/python-jose/tests/test_jwt.py:65: PytestDeprecationWarning: The 'message' parameter is deprecated.
(did you mean to use `match='some regex'` to check the exception message?)
Please comment on https://github.com/pytest-dev/pytest/issues/3974 if you have concerns about removal of this parameter.
with pytest.raises(JWTError, message='Invalid payload string: ["a", "b"}'):
tests/test_jwt.py::TestJWT::test_invalid_claims
/Users/peteryin/code/open_source/python-jose/tests/test_jwt.py:80: PytestDeprecationWarning: The 'message' parameter is deprecated.
(did you mean to use `match='some regex'` to check the exception message?)
Please comment on https://github.com/pytest-dev/pytest/issues/3974 if you have concerns about removal of this parameter.
with pytest.raises(JWTError, message='Invalid payload string: must be a json object'):
-- Docs: https://docs.pytest.org/en/latest/warnings.html
---------- coverage: platform darwin, python 2.7.14-final-0 ----------
Name Stmts Miss Cover Missing
-------------------------------------------------------------
tests/__init__.py 0 0 100%
tests/algorithms/__init__.py 0 0 100%
tests/algorithms/test_EC.py 79 0 100%
tests/algorithms/test_HMAC.py 32 0 100%
tests/algorithms/test_RSA.py 121 1 99% 17
tests/algorithms/test_base.py 11 0 100%
tests/rfc/__init__.py 0 0 100%
tests/rfc/test_rfc7520.py 20 0 100%
tests/test_firebase.py 12 0 100%
tests/test_jwk.py 65 0 100%
tests/test_jws.py 161 0 100%
tests/test_jwt.py 286 0 100%
tests/test_utils.py 9 0 100%
-------------------------------------------------------------
TOTAL 796 1 99%
======================================================================================================================= 184 passed, 2 warnings in 12.12 seconds =======================================================================================================================

Python 3.7.1

(venv3) ➜ python-jose git:(master) ✗ pytest tests
================================================================================================================================= test session starts =================================================================================================================================
platform darwin -- Python 3.7.1, pytest-4.1.1, py-1.5.4, pluggy-0.8.1
rootdir: /Users/peteryin/code/open_source/python-jose, inifile: setup.cfg
plugins: cov-2.6.1
collected 184 items
tests/test_firebase.py ... [ 1%]
tests/test_jwk.py ......... [ 6%]
tests/test_jws.py ...................................... [ 27%]
tests/test_jwt.py ................................................. [ 53%]
tests/test_utils.py .. [ 54%]
tests/algorithms/test_EC.py ......................... [ 68%]
tests/algorithms/test_HMAC.py ... [ 70%]
tests/algorithms/test_RSA.py .................................................. [ 97%]
tests/algorithms/test_base.py .. [ 98%]
tests/rfc/test_rfc7520.py ... [100%]
================================================================================================================================== warnings summary ===================================================================================================================================
tests/test_jwt.py::TestJWT::test_invalid_claims_json
/Users/peteryin/code/open_source/python-jose/tests/test_jwt.py:65: PytestDeprecationWarning: The 'message' parameter is deprecated.
(did you mean to use `match='some regex'` to check the exception message?)
Please comment on https://github.com/pytest-dev/pytest/issues/3974 if you have concerns about removal of this parameter.
with pytest.raises(JWTError, message='Invalid payload string: ["a", "b"}'):
tests/test_jwt.py::TestJWT::test_invalid_claims
/Users/peteryin/code/open_source/python-jose/tests/test_jwt.py:80: PytestDeprecationWarning: The 'message' parameter is deprecated.
(did you mean to use `match='some regex'` to check the exception message?)
Please comment on https://github.com/pytest-dev/pytest/issues/3974 if you have concerns about removal of this parameter.
with pytest.raises(JWTError, message='Invalid payload string: must be a json object'):
-- Docs: https://docs.pytest.org/en/latest/warnings.html
---------- coverage: platform darwin, python 3.7.1-final-0 -----------
Name Stmts Miss Cover Missing
-------------------------------------------------------------
tests/__init__.py 0 0 100%
tests/algorithms/__init__.py 0 0 100%
tests/algorithms/test_EC.py 79 0 100%
tests/algorithms/test_HMAC.py 32 0 100%
tests/algorithms/test_RSA.py 121 0 100%
tests/algorithms/test_base.py 11 0 100%
tests/rfc/__init__.py 0 0 100%
tests/rfc/test_rfc7520.py 20 0 100%
tests/test_firebase.py 12 0 100%
tests/test_jwk.py 65 0 100%
tests/test_jws.py 161 0 100%
tests/test_jwt.py 286 0 100%
tests/test_utils.py 9 0 100%
-------------------------------------------------------------
TOTAL 796 0 100%
======================================================================================================================= 184 passed, 2 warnings in 12.07 seconds =======================================================================================================================

There are some additional deprecation warnings that are coming from pytest, but I decided to stay conservative with the scope of this PR since it's my first contribution to this library.

@codecov

codecovBot commented Jan 15, 2019

Copy link
Copy Markdown

Codecov Report

Merging #124 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #124 +/- ##
==========================================
+ Coverage 94.6% 94.63% +0.03% 
==========================================
Files 13 13 Lines 1000 1006 +6 ==========================================
+ Hits 946 952 +6 
Misses 54 54
Impacted FilesCoverage Δ
jose/jwt.py100% <100%> (ø)⬆️
jose/jws.py96.72% <100%> (+0.08%)⬆️

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 cc710e0...700552a. Read the comment docs.

@peteryin21

peteryin21 commented Jan 15, 2019

Copy link
Copy Markdown
ContributorAuthor

@mpdavis I grew up in Ames. Crazy coincidence!

@peteryin21

Copy link
Copy Markdown
ContributorAuthor

@zejn Tagging you because it looks like you are also a maintainer based on some recently merged PRs. Any tips for getting this PR shipped?

@zejn

zejn commented Apr 9, 2019

Copy link
Copy Markdown
Collaborator

This still looks good. Thank you for your first contribution.

@zejn
zejn merged commit aac4c32 into mpdavis:masterApr 9, 2019
@peteryin21
peteryin21 deleted the import_from_collection_abc branch April 9, 2019 21:59
@kkozmickkozmic mentioned this pull request Dec 6, 2019
CopilotAI mentioned this pull request May 28, 2025
Sign up for freeto 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.

2 participants

@peteryin21@zejn