-
Notifications
You must be signed in to change notification settings - Fork 277
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
NEP 29: bump minimal supported Python version from 3.6 to 3.7 #2917
Conversation
4968959
to
3bc4d3c
Compare
f3fb695
to
d2a70b7
Compare
8d687c1
to
02dcd95
Compare
3075697
to
da2bc1c
Compare
yt/__init__.py
Outdated
) | ||
py_version = sys.version_info | ||
if not (py_version.major == 3 and py_version.minor >= 7): | ||
raise sys.exit("yt requires Python 3.7 or later.") |
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.
I support switching our testing infra to not explicitly test against python 3.6, but I am not sure we should have a hard enforcement here. What about issuing a warning for python < 3.7 that we don't explicitly support those versions and users should expect odd behavior or something?
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.
Ok so I should have stated it earlier: the point is to make our lives easier when we start adding type annotations since their level of support is a bit limited in 3.6
I think it's preferable to have a clean PR that breaks compatibility on purpose, as part of a cycle, so it's clear to everyone we can start supporting 3.9 AND don't have to support 3.6 any longer.
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.
Right, I understand that. I just don't know why we can't choose to say "use at your own risk. This Python version is not explicitly supported in this version of yt" and then users can try to install and use yt with 3.6 if they want to, rather than issuing this more aggressive enforcement.
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.
I don't think it can be done in a way that the message would actually show up before a syntaxError is raised. Otherwise I would support this.
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.
Huh, I didn't realize that we used py 3.7 syntax. Where do we use that?
My preference would be that until we actually move forward with the type annotation syntax that isn't supported, we not hard cancel for py3.6. I say this because we have an unknown set of folks using py36, which was the install default for a long time, and we don't want to have a huge barrier to entry, since upgrading the python version can be confusing. Plus, we might have folks using it with supercomputer python installs, and I think we need to err on the side of forgiveness there.
One thing we should do -- which we are several years behind on doing! -- is take a survey of the community to find out, among other things, what the python version distribution is.
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.
Huh, I didn't realize that we used py 3.7 syntax. Where do we use that?
To be clear: we don't, atm.
I should add that I didn't write this PR with the intention of getting it merged early, it could very well sit here unmerged right until the moment we decide to use Python 3.7+ stuff, such as type annotations with decent support, but I wanted this to be ready when that day comes.
Personally I believe yt 4.0 should still support Python 3.6 but we should be allowed to break it for the following release.
Such a survey would be incredibly useful Matt !
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.
I agree that 4.0 should still support 3.6 -- once we up the release cadence I won't be as worried about dropping python versions but I do worry that dropping 3.6 for 4.0 could cause problems for our users. I also agree with matt that erring on the side of forgiveness for users and not hard cancelling a specific version. I think we need to find a solution that will be forgiving to users.
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.
Agreed. To sum up: this PR will be necessary at some point and contains (or should contain) every change needed when we start annotating code (or using other py3.7+ stuff).
At that point, python 3.6's interpreter won't be able to run import yt
, so I don't think it will be possible to transition with a "use at your own risks" warning.
In the mean time, no rush, we can keep python 3.6 as our oldest supported version.
Note that numpy is in the process of explicitly dropping support for Python 3.6 in there latest release (candidate) |
I have reclassified this with a "doc" label because the main gain is being able to start adding type hints to the code base without the inherent limitation they have in prior to python 3.7 (no forward declaration). |
0d982c7
to
2926711
Compare
6ff2c87
to
a840807
Compare
Note to myself that this should be rebased and include a change in the new static badge for python version |
Considering:
I think we're almost set to merge this. Of course I'd like to cause as little disruption as possible so I propose we agree to merge on a fixed date (end of September, say ?), and warn everyone on official channels well ahead of time, maybe even repeatedly. Any objections ? @matthewturk @cphyc @munkm @Xarthisius @jzuhone @brittonsmith @chummels ? |
6e4c6b9
to
7fd4489
Compare
246632d
to
8f7ec34
Compare
This seems to be relevant still, since we are changing our minimal python version. Is it still the appropriate fix for the issue? |
Yup. Been curating this PR for a year now, so I still think it is necessary. Let's the merge the conflicts now. |
8f7ec34
to
718d25d
Compare
@Xarthisius ptal |
718d25d
to
769ea6b
Compare
769ea6b
to
63802dd
Compare
@yt-fido test this please |
63802dd
to
d481608
Compare
@yt-fido test this please |
d481608
to
9f51fb5
Compare
@yt-fido test this please |
PR Summary
With the upcoming Python 3.9 release, and following the guidelines in https://numpy.org/neps/nep-0029-deprecation_policy.html, we can soon drop support for Python versions older than 3.7.0
This will enable easier introduction of type annotations in yt (see https://docs.python.org/3.7/whatsnew/3.7.html#whatsnew37-pep563)
An important note is that starting from 3.7.0, we can use
to support postponed annotation evaluation (as is standard from Python 3.8.0), see https://www.python.org/dev/peps/pep-0563/#enabling-the-future-behavior-in-python-3-7
This PR should contain the minimal changes to enforce this support drop, but is not meant to be merged before we are able to test against Python 3.9 (most likely not before end of October 2020).
Note: from the changelog in Cython 0.29.1 (https://cython.readthedocs.io/en/latest/src/changes.html#id60)
In a follow up PR I'll bump non minimal test infra from Python 3.8 to 3.9, though this is out of scope for the present PR.
edit : this is done in #2978