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

Partially revert #8042 and add CI in order to allow Python 3.12 support. #8093

Merged

Conversation

WilliamJamieson
Copy link
Collaborator

@WilliamJamieson WilliamJamieson commented Nov 27, 2023

Resolves JP-nnnn

Closes #8044

This PR enables Python 3.12 support for JWST, the main issue preventing this was solved by #8033.

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • Make sure the JIRA ticket is resolved properly

@WilliamJamieson WilliamJamieson requested a review from a team as a code owner November 27, 2023 16:22
@github-actions github-actions bot added testing installation automation Continuous Integration (CI) and testing automation tools labels Nov 27, 2023
@tapastro
Copy link
Contributor

If/when/as this PR is merged, we can also revert #8057

@WilliamJamieson WilliamJamieson marked this pull request as draft November 27, 2023 16:29
@WilliamJamieson
Copy link
Collaborator Author

Looks like jwst still uses pkg_resources. I'll open a separate PR fixing that problem. The devdeps failure looks real, but due to numpy 2.0 problems.

@WilliamJamieson
Copy link
Collaborator Author

This requires #8095 in order to function properly as pkg_resources has been removed by Python 3.12.

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (870f9de) 75.94% compared to head (2311764) 75.94%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8093   +/-   ##
=======================================
  Coverage   75.94%   75.94%           
=======================================
  Files         460      460           
  Lines       37625    37629    +4     
=======================================
+ Hits        28573    28577    +4     
  Misses       9052     9052           
Flag Coverage Δ *Carryforward flag
nightly 77.37% <ø> (ø) Carriedforward from 870f9de

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/ci.yml Show resolved Hide resolved
- linux: py311-stdevdeps-xdist
- linux: py311-devdeps-xdist
- linux: py311-xdist-cov
- linux: py312-stdevdeps-xdist
Copy link
Collaborator

Choose a reason for hiding this comment

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

So these various entries look to me like we're keeping tests using python 3.9 and 3.10, but getting rid of all tests using 3.11 and replacing them with 3.12? Why aren't we keeping any 3.11 tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Line 72 runs the python 3.11 tests. If you would prefer I could just keep it Python 3.11 for the macos and stdevdeps jobs, only adding the additional py312-xdist job? I updated these because in the past we seem to have updated them to the latest version of python supported.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that we'll be delivering our next build under 3.11, I'd prefer to keep both the Linux and Macos entries using 3.11, but then use 3.12 for everything that involves latest/dev dependencies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, it now just adds a 3.12 linux job to the standard CI and changes the devdeps related jobs to 3.12

@WilliamJamieson WilliamJamieson force-pushed the feature/enable_python_312 branch from 7604d41 to b5b8525 Compare November 30, 2023 16:13
@WilliamJamieson WilliamJamieson marked this pull request as ready for review November 30, 2023 16:14
@WilliamJamieson WilliamJamieson force-pushed the feature/enable_python_312 branch from a40d386 to 533c09b Compare December 1, 2023 14:41
@WilliamJamieson WilliamJamieson force-pushed the feature/enable_python_312 branch from 533c09b to 9b6f1f3 Compare December 1, 2023 14:42
Copy link
Collaborator

@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

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

LGTM

@hbushouse hbushouse merged commit f13f8f5 into spacetelescope:master Dec 5, 2023
23 of 24 checks passed
@tapastro
Copy link
Contributor

tapastro commented Dec 7, 2023

Merging this should maybe have prompted a build - the readme is now updated to imply support of python 3.12, but no release supports 3.12 yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation Continuous Integration (CI) and testing automation tools documentation installation jump ramp_fitting testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add python 3.12 support
3 participants