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

SPEC 0: drop py310 and support py313 #6195

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

stephenworsley
Copy link
Contributor

@stephenworsley stephenworsley commented Oct 25, 2024

🚀 Pull Request

Description

closes #6177

Comment on lines 46 to 49
],
"44fae030": [
"ENV_PARENT={conf_dir}/.asv/env/nox310",
"PY_VER=3.10 nox --envdir={env_parent} --session=tests --install-only --no-error-on-external-run --verbose"
Copy link
Contributor

@trexfeathers trexfeathers Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I would prefer that we keep the old one too, as we occasionally have need to benchmark further back in history.

@@ -37,16 +37,16 @@
],
"delegated_env_commands": {
"c8a663a0": [
"ENV_PARENT={conf_dir}/.asv/env/nox313",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awkward - we don't know what the new commit hash will be once this is merged. I didn't think of that 🙄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with @ESadek-MO: if you just provide one of the commit hashes that is on your branch, and then we'll merge this one with a merge-commit (not a squash).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like this bd6495c ?

Copy link
Contributor

@trexfeathers trexfeathers Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost, however there are now duplicate 3.13, which is what is causing the benchmark failure. The sequence should be 3.10, 3.11, 3.12, 3.13 - unlike other things we are not describing CI coverage, but support for benchmarking older commits if requested.

@github-actions github-actions bot added the benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts label Oct 25, 2024
@stephenworsley stephenworsley marked this pull request as ready for review October 25, 2024 14:08
@stephenworsley
Copy link
Contributor Author

Looks like python 3.13 is causing a lot of test failures. Most of these seem to be in cml files where I'm seeing &#10 show up a lot. I think this may be due to how python is rendering newlines. Seems like this can't be fixed by rerendering all the cml files since that would break the tests for the older versions so this seems like a bit of a blocker for the moment.

@stephenworsley
Copy link
Contributor Author

Looks like this change might be the cause of the failing tests, python/cpython#107947 .

@valeriupredoi
Copy link

hi folks, we are ready to roll out support for ESMValCore and Python 3.13 in ESMValGroup/ESMValCore#2566 - @bouweandela brought this issue to my attention, do we understand that iris==3.11 doesn't really-really support Python 3.13 and we have to wait for iris==3.12 or an eventual bugfix iris==3.11.1? Cheers muchly! 🍺

@trexfeathers
Copy link
Contributor

hi folks, we are ready to roll out support for ESMValCore and Python 3.13 in ESMValGroup/ESMValCore#2566 - @bouweandela brought this issue to my attention, do we understand that iris==3.11 doesn't really-really support Python 3.13 and we have to wait for iris==3.12 or an eventual bugfix iris==3.11.1? Cheers muchly! 🍺

The test failures are all caused by a change in XML character representation, which stops us comparing against known-good-outputs. This has made 5% of our tests 'blind' until we can work on a solution. But we have NO evidence of any Iris functionality breaking, and 95% of our tests still work and they show Iris functioning fine. Your own passing tests (ESMValGroup/ESMValCore#2566 (comment)) back this up even more.

So the Iris 'project' cannot support Python 3.13 until we can get our CI working with 3.13; we have no immediate plans for finishing this work as we need space to finish some other things. But it looks like the Iris software supports 3.13 just fine already 👍. This happens with all new Python versions - our users have success with it before we even get a chance to test against it.

@valeriupredoi
Copy link

hi folks, we are ready to roll out support for ESMValCore and Python 3.13 in ESMValGroup/ESMValCore#2566 - @bouweandela brought this issue to my attention, do we understand that iris==3.11 doesn't really-really support Python 3.13 and we have to wait for iris==3.12 or an eventual bugfix iris==3.11.1? Cheers muchly! 🍺

The test failures are all caused by a change in XML character representation, which stops us comparing against known-good-outputs. This has made 5% of our tests 'blind' until we can work on a solution. But we have NO evidence of any Iris functionality breaking, and 95% of our tests still work and they show Iris functioning fine. Your own passing tests (ESMValGroup/ESMValCore#2566 (comment)) back this up even more.

So the Iris 'project' cannot support Python 3.13 until we can get our CI working with 3.13; we have no immediate plans for finishing this work as we need space to finish some other things. But it looks like the Iris software supports 3.13 just fine already 👍. This happens with all new Python versions - our users have success with it before we even get a chance to test against it.

wonderful, exactly the answer I was looking for, both wrt clarity and content, cheers, Martin! 🍺
@bouweandela we're good to go with ESMValGroup/ESMValCore#2566 then 🧨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Support Python 3.13
3 participants