-
Notifications
You must be signed in to change notification settings - Fork 279
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
Cleanup legacy python checks #2641
Cleanup legacy python checks #2641
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good!
Since this PR unvendors lru_cache I'd delete references to it (like in setup.cfg
, .coveragerc
, and .pep8speaks
) too.
Thanks ! Indeed I didn't think it was mentioned anywhere explicitly. |
485bd36
to
60a7be9
Compare
d684a72
to
3c4f2f5
Compare
3c4f2f5
to
ce68d3e
Compare
There are many places in the code base where we check the python version. 99% of the time it's done to handle special cases where python 2 and python 3 have different behaviours. Since
yt/__init__.py
now raises an exception when python 2 is used, I think there is no question that those other checks should be removed.However there are very rare (two) occurrences where we check for versions more recent than 3.0 (namely 3.3 and 3.4). Since we do not test against any version older than 3.6, and a vast majority of the python3 community is using 3.6 or later (3% are using 3.5 and another 3% is using older versions) (source), I assume it's not an issue to remove those checks too.
In particular, this removes the need for
yt.utilities.lru_cache
which is a backport of std lib meant for python 3.2 and older.I think it'd be good to explicitly state somewhere what minor version of python 3.x we're supporting (is it 3.5 or 3.6 ?). It could also be checked in init.py for safety.