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

Import collections ABCs from collections.abc, not collections #2683

Merged
merged 1 commit into from
Apr 9, 2019

Conversation

sd2k
Copy link
Contributor

@sd2k sd2k commented Apr 2, 2019

This fixes a DeprecationWarning which would have broken in Python 3.8:

/home/ben/.virtualenvs/monitoring/lib/python3.7/site-packages/luigi/parameter.py:29
  /home/ben/.virtualenvs/monitoring/lib/python3.7/site-packages/luigi/parameter.py:29: 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, Mapping

/home/ben/.virtualenvs/monitoring/lib/python3.7/site-packages/luigi/scheduler.py:211
  /home/ben/.virtualenvs/monitoring/lib/python3.7/site-packages/luigi/scheduler.py:211: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
    class OrderedSet(collections.MutableSet):

Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

@sd2k We still support Python 2.7. To remove these warnings, we'll need to change the import based on the python version

Thanks for working to remove these warnings!

This fixes a DeprecationWarning which would have broken in Python 3.8.
@sd2k
Copy link
Contributor Author

sd2k commented Apr 2, 2019

@dlstadther Ah, of course! Thanks for checking. I've wrapped the imports in a try/except block now, and tests are passing.

@Tarrasch Tarrasch merged commit c0316de into spotify:master Apr 9, 2019
@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 9, 2019

Thanks!!

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.

3 participants