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

NEP 29: bump minimal supported Python version from 3.6 to 3.7 #2917

Merged
merged 1 commit into from
Oct 21, 2021

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Sep 18, 2020

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

from __future__ import annotations

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)

The exception handling in generators and coroutines under CPython 3.7 was adapted to the newly introduced exception stack. Users of Cython 0.28 who want to support Python 3.7 are encouraged to upgrade to 0.29 to avoid potentially incorrect error reporting and tracebacks. (Github issue #1958)

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

@neutrinoceros neutrinoceros added enhancement Making something better infrastructure Related to CI, versioning, websites, organizational issues, etc labels Sep 18, 2020
@neutrinoceros neutrinoceros marked this pull request as draft September 18, 2020 15:36
yt/__init__.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
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.")
Copy link
Member

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?

Copy link
Member Author

@neutrinoceros neutrinoceros Nov 24, 2020

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.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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 !

Copy link
Member

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.

Copy link
Member Author

@neutrinoceros neutrinoceros Nov 24, 2020

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.

@neutrinoceros
Copy link
Member Author

Note that numpy is in the process of explicitly dropping support for Python 3.6 in there latest release (candidate)
https://github.com/numpy/numpy/releases/tag/v1.20.0rc2

@neutrinoceros neutrinoceros changed the title Drop support for python 3.6 type hints: drop support for python 3.6 Jan 20, 2021
@neutrinoceros
Copy link
Member Author

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).

@neutrinoceros
Copy link
Member Author

neutrinoceros commented Feb 5, 2021

Note to myself that this should be rebased and include a change in the new static badge for python version
edit: ✅

@neutrinoceros
Copy link
Member Author

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 ?

@matthewturk
Copy link
Member

This seems to be relevant still, since we are changing our minimal python version. Is it still the appropriate fix for the issue?

@neutrinoceros
Copy link
Member Author

Yup. Been curating this PR for a year now, so I still think it is necessary. Let's the merge the conflicts now.

@matthewturk
Copy link
Member

@Xarthisius ptal

@neutrinoceros
Copy link
Member Author

@yt-fido test this please

@neutrinoceros
Copy link
Member Author

@yt-fido test this please

@neutrinoceros neutrinoceros linked an issue Oct 19, 2021 that may be closed by this pull request
@neutrinoceros
Copy link
Member Author

@yt-fido test this please

@munkm munkm merged commit 6df9119 into yt-project:main Oct 21, 2021
@neutrinoceros neutrinoceros deleted the drop_py36 branch October 21, 2021 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement Making something better infrastructure Related to CI, versioning, websites, organizational issues, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Following NEP on Python version support
5 participants