-
Notifications
You must be signed in to change notification settings - Fork 434
MTN update ci pipelines to use python3.8 + test more pypy versions #1487
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1487 +/- ##
==========================================
+ Coverage 95.08% 95.09% +0.01%
==========================================
Files 44 44
Lines 7564 7564
==========================================
+ Hits 7192 7193 +1
+ Misses 372 371 -1 ☔ View full report in Codecov by Sentry. |
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.
LGTM, thanks
So this is not just fixing CI, it completely drops support for python 3.7. I'm not sure we want to do that for a bug fix release. To me if we wanted to drop support for 3.7 we should have done that for 1.3.0. Now we're kind of stuck with it until 1.4.0, don't you think ? |
Now we're kind of stuck with it until 1.4.0, don't you think ?
Indeed, I agree with you. And we do need to do a bugfix release. So I guess we should do this release ASAP.
|
The first commit in this branch contains the bugfix: d75cd9e I can open a separate PR with this commit only if it is preferable.
If bug fix releases are needed it can start from a branch starting from the latest commit release, that keeps the 3.7 pipelines + apply the workaround to the more_itertools problem to go on. Depends how you want to manage releases. Applying the workaround on master means one has to remember reverting it at some point. |
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 I understand correctly, the released version is compatible with python 3.7 and the bug fix release is too. But pytest
is not so this breaks our pipelines.
I would say, we fix the pipelines with this PR as a maintenance commit by bumping up the version we test on. Then we do a bug fix release and drop python 3.7 afterward?
To me, the only changes relative to dropping python 3.7 are the changes in the setup. I would not overcomplicate things by making a temporary pin for pytest version as the current code has already been tested on python3.7 extensively but if you feel this is mandatory, we can do it.
but let's do the bf release soon!
- Add more pypy pipelines to test matrix + oldest supported python (3.8) + oldest compatible pypy version (7.3.6) + oldest supported python (3.8) + most recent compatible pypy version (7.3.11) + most recent pypy version (7.3.12) + most recent supported python version (3.10) Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>
Do we really want to support several PyPy versions? I would personally be find just testing on the most recent stable release of PyPy. |
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.
Seems like we forgot that we already had a PR to drop 3.7 and did the change in #1515...
Sorry about that.
This PR is still worth merging as it improves the support for pypy.
Just saw this comment, I have -0 opinion on this. As the pipelines are already quite long, there is maybe a fair point in only trying the latest version. So this would mean closing this PR as this is the main change in the end. |
I thought this PR was closing so I un-assigned. Should we still push it forward ? The last pipeline fails with a deadlock with the multiprocessing backend, I've never seen it before, probably rarely occuring bug. Shouldn't we also bump this pipeline to |
The readthedocs pipeline failure comes from expired certificate on docs.scipy.org Relevant issue scipy/docs.scipy.org#80 I added configuration to bypass the certificate in the readthedoc pipeline. It's green again. But it sounds better to just wait for scipy/docs.scipy.org#80 to be solved before continuing forward, all the more so since sphinx doesn't support setting |
Yes, let's revert the |
The package more_itertools, which is a dependency of pluggy/pytest, has lost compatibility with python 3.7 when releasing the version more_itertools>=10, and the conda distributions for more_itertools don't embed version specifiers for compatibility with python so the conda resolver installs version 10 anyway in the python 3.7 pipelines.
The pipelines with python3.7 can be fixed by requiring "more_itertools<10" in the pipeline setup script here and here
However python 3.7 also recently reached end of life so rather than working around this issue, this PR proposes to bump the python3.7 pipelines to python3.8 (and also bump numpy==1.15 to numpy==1.16 which is the smallest version that supports python 3.8, much like 1.15 was the smallest version that supported python 3.7)