Skip to content

[Build | CI] Python 3 12 support#374

Merged
fmalatino merged 3 commits intoNOAA-GFDL:developfrom
FlorianDeconinck:build_ci/python_3_12_support
Feb 3, 2026
Merged

[Build | CI] Python 3 12 support#374
fmalatino merged 3 commits intoNOAA-GFDL:developfrom
FlorianDeconinck:build_ci/python_3_12_support

Conversation

@FlorianDeconinck
Copy link
Collaborator

Description

Python 3.12 support was locked behind a leaking of thread_local variable. It was followed back all the way to a pybind11 issue with 3.x versions. GT4Py is now limiting to the use of pybind11 to 2.x.

Now cleared out of this bug, gt4py and dace already support and run 3.12 as part of their CI. We do not expect issues with NDSL.

This PR updates the maximum python version to 3.12 and change the NDSL unit tests to be run on both versions.

Also:

  • This updates GT4Py and by knock-on effect bring K query in the numpy backend to NDSL.

How has this been tested?

Let the CI be the judge !

@fmalatino
Copy link
Contributor

Approved pending CI passes

Copy link
Collaborator

@romanc romanc left a comment

Choose a reason for hiding this comment

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

PR looks good to me. Required GHA checks are done by name. And since this name changed, this PR will be stuck until that requirement is fullfilled (or the restriction is lifted). I suggest diabling the "ndsl unit test" requirements temporarily. In the long run, we probably want to change blocking CI to have a skngle entey point such that we can change what is blocking without access to the repo's admin settings (where this is currently configured).

@FlorianDeconinck
Copy link
Collaborator Author

PR looks good to me. Required GHA checks are done by name. And since this name changed, this PR will be stuck until that requirement is fullfilled (or the restriction is lifted). I suggest diabling the "ndsl unit test" requirements temporarily. In the long run, we probably want to change blocking CI to have a skngle entey point such that we can change what is blocking without access to the repo's admin settings (where this is currently configured).

Poke @fmalatino can you deactivate the requirements of "ndsl unit test" please? We don't have access.

@fmalatino fmalatino enabled auto-merge February 3, 2026 15:39
@fmalatino fmalatino added this pull request to the merge queue Feb 3, 2026
Merged via the queue into NOAA-GFDL:develop with commit 7b516a8 Feb 3, 2026
7 checks passed
@romanc
Copy link
Collaborator

romanc commented Feb 4, 2026

Now that this PR is merged, @fmalatino can you make sure that NDSL unit tests @ python 3.11 and 3.12 are both required for merge again?

image

Thanks! 🙏

@fmalatino
Copy link
Contributor

Now that this PR is merged, @fmalatino can you make sure that NDSL unit tests @ python 3.11 and 3.12 are both required for merge again?

image Thanks! 🙏

It should be now. Let me know if it does not appear on your end.

@romanc
Copy link
Collaborator

romanc commented Feb 6, 2026

Now that this PR is merged, @fmalatino can you make sure that NDSL unit tests @ python 3.11 and 3.12 are both required for merge again?
image
Thanks! 🙏

It should be now. Let me know if it does not appear on your end.

@fmalatino, It still looks the same on my side (e.g. in PRs #377 and #372)

image

This needs to be configured in the admin settings of the repo where I don't have access to. It's part of the branch protection rules. Here's a screenshot (from another repo) what to look out for

image

in that list of "Status checks that are required" you should see the old one, which needs to be removed. And in the same list, you can add the new ones (there are now two new ones, one for python 3.11 and one for python 3.12). Those should be required and thus added to the list.

@fmalatino
Copy link
Contributor

Now that this PR is merged, @fmalatino can you make sure that NDSL unit tests @ python 3.11 and 3.12 are both required for merge again?
image
Thanks! 🙏

It should be now. Let me know if it does not appear on your end.

@fmalatino, It still looks the same on my side (e.g. in PRs #377 and #372)

image This needs to be configured in the admin settings of the repo where I don't have access to. It's part of the branch protection rules. Here's a screenshot (from another repo) what to look out for image in that list of "Status checks that are required" you should see the old one, which needs to be removed. And in the same list, you can add the new ones (there are now two new ones, one for python 3.11 and one for python 3.12). Those should be required and thus added to the list.

Please let me know if they appear correct now.

@romanc
Copy link
Collaborator

romanc commented Feb 6, 2026

Now that this PR is merged, @fmalatino can you make sure that NDSL unit tests @ python 3.11 and 3.12 are both required for merge again?

Thanks! 🙏

It should be now. Let me know if it does not appear on your end.

@fmalatino, It still looks the same on my side (e.g. in PRs #377 and #372)

This needs to be configured in the admin settings of the repo where I don't have access to. It's part of the branch protection rules. Here's a screenshot (from another repo) what to look out for

in that list of "Status checks that are required" you should see the old one, which needs to be removed. And in the same list, you can add the new ones (there are now two new ones, one for python 3.11 and one for python 3.12). Those should be required and thus added to the list.

Please let me know if they appear correct now.

It all looks good now. PRs proceed to the merge queue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants