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

FIX: Fix Python 3.8 deprecation of ast.Num in favor of ast.Constant #257

Merged
merged 5 commits into from Dec 26, 2019

Conversation

@bocklund
Copy link
Collaborator

bocklund commented Dec 18, 2019

Accoring to the Python 3.8 ast docs:

Deprecated since version 3.8: Class ast.Constant is now used for all constants. Old classes ast.Num, ast.Str, ast.Bytes, ast.NameConstant and ast.Ellipsis are still available, but they will be removed in future Python releases.

The tdb.py module whitelists certain ast objects, including ast.Num, which are now ast.Constant, causing breakages in sympifying parsed strings. This adds ast.Constant to our whitelist.

Thanks @jwsiegel2510 for pointing this out at PhasesResearchLab/ESPEI#110

As an aside: this is an interesting breakage because ast.Num still exists in Python 3.8, but instantiating an ast.Num actually gives you an ast.Constant instance, which breaks the whitelist even though people creating objects with ast.Num in Python 3.8 are not technically affected.

@bocklund

This comment has been minimized.

Copy link
Collaborator Author

bocklund commented Dec 18, 2019

Note that ast.Constant has existed since Python 3.6, so merging this commit would effectively drop Python 2.7 support. Since Python EOL is January 1 and many of our dependencies are no longer supporting Python 2.7, I'm okay with pycalphad 0.8.1.post1 being the last Python 2.7 release.

@bocklund bocklund force-pushed the py38-fix-tdb branch from c2923fd to c423dd5 Dec 19, 2019
@bocklund

This comment has been minimized.

Copy link
Collaborator Author

bocklund commented Dec 19, 2019

Rebased onto develop after merging #252 to verify that the changes fix Python 3.8 and all the tests pass on CI. Note the failing Python 2.7 tests per my comment above.

@bocklund bocklund requested a review from richardotis Dec 19, 2019
Copy link
Collaborator

richardotis left a comment

I agree with dropping Python 2.7 support. I see that we haven't tested Python 3.5 for a while either; did we drop py35 support already, or will this commit do it?

@bocklund

This comment has been minimized.

Copy link
Collaborator Author

bocklund commented Dec 24, 2019

We haven't been testing 3.5 since we dropped support along with conda-forge (#211). I think conda-forge is only supporting the latest two versions of Python, so any compatibility we had with Python 3.5 was unofficial.

The latest changes drop Python 2.7 from the tests and removes most of the obvious Python 2 vs. Python 3 specific code. I'll merge after the tests are passing

@bocklund bocklund force-pushed the py38-fix-tdb branch 2 times, most recently from 0b801b5 to cb81731 Dec 24, 2019
@bocklund bocklund force-pushed the py38-fix-tdb branch from cb81731 to fc5339b Dec 24, 2019
bocklund added 2 commits Dec 24, 2019
@bocklund bocklund force-pushed the py38-fix-tdb branch from 2dee88a to aceb8df Dec 26, 2019
@richardotis

This comment has been minimized.

Copy link
Collaborator

richardotis commented Dec 26, 2019

The coverage change is noise in comparison to the coverage difference between the Python versions we test. You can merge anyway.

@bocklund bocklund merged commit c7a275b into develop Dec 26, 2019
4 of 5 checks passed
4 of 5 checks passed
coverage/coveralls Coverage decreased (-0.07%) to 86.086%
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@bocklund bocklund added this to the 0.8.2 milestone Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.