Skip to content

Fix collections.abc DeprecationWarning in sample.py - #936

Merged
dwhswenson merged 2 commits into
openpathsampling:masterfrom
sroet:fix_py_39_depr
Oct 17, 2020
Merged

dwhswenson merged 2 commits into
openpathsampling:masterfrom
sroet:fix_py_39_depr

Conversation

@sroet

@sroet sroet commented Oct 17, 2020

Copy link
Copy Markdown
Member

Going from 5570cd3 to ace9d7e and running the tests surfaced another

/home/sroet/github_files/openpathsampling/openpathsampling/sample.py:10: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working
    from collections import Counter, Mapping

This solves that one (Counter still has to be imported from base collections)

@sroet sroet changed the title solve another depr warning [review] fix another depr warning Oct 17, 2020
@dwhswenson

Copy link
Copy Markdown
Member

I think you're gonna need a try: except: for Python 2.

@sroet

sroet commented Oct 17, 2020

Copy link
Copy Markdown
Member Author

Yeah, figured that one out. Decided to explicitly check for python3 so it is easier to remove if/when OPS drops python 2.7

@dwhswenson

Copy link
Copy Markdown
Member

Sure... I usually add a comment with the string Py2 or something similar, so feel free to also do that if you come across others of this. Explicit version check is also fine here.

@dwhswenson dwhswenson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good. Will merge on green.

@dwhswenson dwhswenson changed the title [review] fix another depr warning Fix collections.abc DeprecationWarning in sample.py Oct 17, 2020
@dwhswenson
dwhswenson merged commit 7c7cf7c into openpathsampling:master Oct 17, 2020
@sroet
sroet deleted the fix_py_39_depr branch October 17, 2020 11:55
@dwhswenson dwhswenson mentioned this pull request Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants