-
Notifications
You must be signed in to change notification settings - Fork 34
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
Support Python 3.11, drop older Pythons entirely #59
Conversation
tox.ini
Outdated
py{27,34,35,36,37,38,39,310}-wrapt{1.10,1.11,1.12,1.13} | ||
pypy, pypy3 | ||
py{37,38,39,310}-wrapt{1.10,1.11,1.12,1.13,1.14} | ||
py{311}-wrapt{1.14} |
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.
1.14 is needed to get Python 3.11 working GrahamDumpleton/wrapt#196
https://app.travis-ci.com/github/tantale/deprecated/jobs/591380170#L212-L213 indicates that the 3.7 build needs an updated version of the import_metadata library. IIRC this necessity goes away in 3.8 or newer… |
This may need to go into install deps…
Also fix error in pypy3 selection
5f83774
to
d5926ba
Compare
At this point, all Travis x86 jobs are completed… |
AppVeyor can't seem to find a python.exe in the path specified for Python 3.9 or later, so let's explicitly set the image version just in case somehow an older image is being used— AppVeyor doesn't say which image is in use on a build's page…?
I have no idea what I'm doing but I glanced at the docs in [this][1] and [this][2] and feel like I'm on the right track to silencing the warnings. [1]: https://packit.dev/posts/copr-srpms/#deployment-phases [2]: https://packit.dev/docs/configuration/#srpm_build_deps
Travis build failures are transient, it seems. Those failing jobs have succeeded in the past with no relevant changes in the meantime. |
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.
Looks good, especially the wrapt >= 1.14.0
requirement for Python 3.11+, without which is causing problems for users of other libraries which depend on Deprecated:
Some minor suggestions added.
.travis.yml
Outdated
install: | ||
- pip install coveralls tox-travis | ||
# TODO: Remove tox limitation when https://github.com/tox-dev/tox-travis/pull/160 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Other | ||
----- | ||
|
||
- Add support for Python 3.11. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
setup.py
Outdated
'configparser < 5 ; python_version < "3"', | ||
'sphinxcontrib-websupport < 2 ; python_version < "3"', | ||
'zipp < 2 ; python_version < "3"', | ||
'importlib-metadata' |
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.
Is this library used anywhere?
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.
Yes. It's still required for Python 3.7. It can be removed when 3.7 support goes away, which I'd recommend doing when 3.7 is EOL later this year.
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 didn't actually see it used in the code?
Anyway, one suggestion is to use the stdlib version for 3.8+:
'importlib-metadata; python_version < "3.8"',
try:
# Python 3.8+
import importlib.metadata as importlib_metadata
except ImportError:
# Python 3.7
import importlib_metadata # type: ignore
And then it can be removed when 3.7 is EOL in June: https://devguide.python.org/versions/
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 failed to document why I added it in 1827126 but I did note that it is needed for dev only. I think maybe wrapt needed it?
$ rg importlib
setup.py
187: 'importlib-metadata'
$ rg metadata
setup.py
187: 'importlib-metadata'
What's the danger of leaving it now and coming back to removing it? It's a dev dep so it shouldn't affect the public dependencies list.
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.
Ya know, I'll bet I had it in there to consolidate the lines removed below, assuming that importlib-metadata
was actually used somewhere…
py{37,38,39,310}-wrapt{1.10,1.11,1.12,1.13,1.14} | ||
py{311,312}-wrapt{1.14} |
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.
Do we need to test five versions of wrapt? 1.10 is from 2017, pretty old by now.
If we're adding two, shall we remove two?
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.
Support table:
Wrapt version° | Pythons supported |
---|---|
1.10.x | 2.6, 2.7, 3.3, 3.4, 3.5, 3.6 |
1.11.x | 2.6, 2.7, 3.3, 3.4, 3.5, 3.6, 3.7 |
1.12.x | 2.7, 3.4, 3.5, 3.6, 3.7, 3.8 |
1.13.x | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10 |
1.14.x | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11 |
1.15.x rc | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11 |
It looks very safe to drop 1.10 because none of its supported versions would be supported in the next Deprecated release if this PR is merged. I'd call it safe enough to drop 1.11-1.12 and probably even 1.13, tbh.
°: latest in series
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'd say let's keep the wrapt dep and testing at 1.10+ for this PR and if I have time this week (long shot), I can do the work to eliminate 1.10-1.13 to settle on 1.14+.
setup.py
Outdated
'configparser < 5 ; python_version < "3"', | ||
'sphinxcontrib-websupport < 2 ; python_version < "3"', | ||
'zipp < 2 ; python_version < "3"', | ||
'importlib-metadata' |
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.
Yes. It's still required for Python 3.7. It can be removed when 3.7 support goes away, which I'd recommend doing when 3.7 is EOL later this year.
py{37,38,39,310}-wrapt{1.10,1.11,1.12,1.13,1.14} | ||
py{311,312}-wrapt{1.14} |
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.
Support table:
Wrapt version° | Pythons supported |
---|---|
1.10.x | 2.6, 2.7, 3.3, 3.4, 3.5, 3.6 |
1.11.x | 2.6, 2.7, 3.3, 3.4, 3.5, 3.6, 3.7 |
1.12.x | 2.7, 3.4, 3.5, 3.6, 3.7, 3.8 |
1.13.x | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10 |
1.14.x | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11 |
1.15.x rc | 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11 |
It looks very safe to drop 1.10 because none of its supported versions would be supported in the next Deprecated release if this PR is merged. I'd call it safe enough to drop 1.11-1.12 and probably even 1.13, tbh.
°: latest in series
py{37,38,39,310}-wrapt{1.10,1.11,1.12,1.13,1.14} | ||
py{311,312}-wrapt{1.14} |
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'd say let's keep the wrapt dep and testing at 1.10+ for this PR and if I have time this week (long shot), I can do the work to eliminate 1.10-1.13 to settle on 1.14+.
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
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.
Looks good, thank you!
@tantale I'd love your feedback and to see this merged and released. |
@tantale What feedback do you have? |
Any news? I just did the same work without seeing this. |
Hello, Sorry for the (very) late answer: I don’t have much time to work on this project. Anyway, you made a good job. I agree that the dependencies and GitHub actions must be updated. But, since the code base don’t change, there is no good reason to drop support for old Python versions: stating that Pytest can’t test Python 2.7 anymore doesn’t mean the code base is not Python 2.7 compatible. Do you understand the idea? I will find time to review your code. |
Thank you kindly, @laurent-laporte-pro.
I think it's matter of representing what's tested. If the project represents that it's compatible with a version, it should be tested on it, esp. automated testing. Older Deprecated library versions remain compatible; only future releases could push someone toward a non-EOL Python version, a healthy practice for the ecosystem. I think the changes I've made should facilitate re-adding the versions dropped. I could re-add the older versions to the testing matrix, but that could use a lot of CPU time for testing for Python versions that are insecure, out-of-date, and generally unused (with the exception of 2.7, which still has holdouts). For me, getting Python 3.11 supported and tested is a top priority. |
Perhaps this PR can be more focused about adding tests against Python 3.11. The parts about removing older Python support can be done as a separate PR. Or is there reason that we need to do it all at once? |
This PR started as "coopt the existing Python 3.11 support patch and make it work" which turned into "drop the versions with tests that don't pass because of old CI setups" and settled on "hugely revised CI setup that drops old, unsupported versions and adds the new versions". I could re-add the older versions to CI but I really don't want to spend much time in doing so beyond adding their strings to some lists in config files. I sank an afternoon into what's here as of 450dd3c. |
GitHub Actions has also dropped support for EOL Python 3.5-3.6:
https://github.com/hugovk/deprecated/actions/runs/4480549799 |
'Programming Language :: Python :: 2', | ||
'Programming Language :: Python :: 2.7', | ||
'Programming Language :: Python :: 3', | ||
'Programming Language :: Python :: 3.4', | ||
'Programming Language :: Python :: 3.5', | ||
'Programming Language :: Python :: 3.6', |
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.
Since the base code of this library has no change, I don't wan't to drop those versions.
python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*', | ||
python_requires='>=3.7', |
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.
No, I want to keep this unchanged.
From my point of view, the reason that it is no longer possible to run unit tests with the recent tools is not a reason to abandon the support of Python 2.7 and the older versions of Python 3. If a user has an old development environment, he will be able to install and use the library.
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.
But surely they could continue using the older releases of this package, if they still rely on python 2.7?
It won't break any user if they are stuck on an older version, because other packages are also not moving and there is no support/security-fixes anyways?
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 PR does not change the code base, so the code remains compatible with older versions of Python. I don't want to make a major release for this PR: it's only a build fix.
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.
If you're OK with allowing installation on an untested Python version, I'll revert the versions drop. I myself would not want to receive reports about any future failure on untested and EOL versions but I recognize the risk of breakage is low given that this library doesn't need to change much.
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.
Researching a little deeper, I could retain testing on the older versions by switching the base platform for testing to
- ubuntu-20.04 for 3.6 & 3.5
- ubuntu-18.04 for 3.4
- simply re-add 2.7 because there's a package for ubuntu-22.04 (which is ubuntu-latest) for 2.7.18
all of these per https://github.com/actions/python-versions/blob/main/versions-manifest.json
Looks like I can also enable 3.12.0-alpha - 3.12.0
through the semver hyphen ranges.
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.
2.7 is being removed in a couple of weeks actions/runner-images#7401
Instead of 3.12.0-alpha - 3.12.0
, we can use the simpler 3.12-dev
. https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#using-the-python-version-input
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.
Researching a little deeper, I could retain testing on the older versions by switching the base platform for testing to
ubuntu-20.04 for 3.6 & 3.5
ubuntu-18.04 for 3.4
simply re-add 2.7 because there's a package for ubuntu-22.04 (which is ubuntu-latest) for 2.7.18
all of these per https://github.com/actions/python-versions/blob/main/versions-manifest.json
Looks like I can also enable
3.12.0-alpha - 3.12.0
through the semver hyphen ranges.
I am OK with that.
Thanks! Sorry I couldn't get around to massaging this PR to the desired state. It's been a busy few weeks. |
This includes #58 and then drops Pythons older than 3.7 since they're all EOL. It also updates some GHA action versions.