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

reorganize tests, make devdeps testing optional, unpin latest Python testing, and prepare for possibility of data caching #7910

Merged
merged 12 commits into from
Dec 6, 2023

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Sep 18, 2023

Resolves SCSB-79

This PR

  • reorganizes CI tests and moves devdeps tests to a new optional workflow that can be started with the label run devdeps tests
  • similarly, the label run scheduled tests can be used as well
  • pins the version of the OpenAstronomy reusable test workflow to the v1 tag to minimize surprises with API changes
  • abstracts retrieval of the CRDS context (spacetelescope/crds/.github/workflows/contexts.yml@master) to improve maintainability of this workflow going forward
  • caches CRDS data to /tmp/data/ like in other repos' data.yml workflows (can potentially be packaged into a combined cache)
  • run tests on latest Python version in devdeps (py3)

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

@zacharyburnett zacharyburnett self-assigned this Sep 18, 2023
@github-actions github-actions bot added the automation Continuous Integration (CI) and testing automation tools label Sep 18, 2023
@zacharyburnett zacharyburnett changed the title reorganize tests and prepare for future inclusion of data caching reorganize tests, make devdeps testing optional, and prepare for future inclusion of data caching Sep 18, 2023
@zacharyburnett zacharyburnett changed the title reorganize tests, make devdeps testing optional, and prepare for future inclusion of data caching reorganize tests, make devdeps testing optional, and prepare for possibility of data caching Sep 18, 2023
@zacharyburnett zacharyburnett changed the title reorganize tests, make devdeps testing optional, and prepare for possibility of data caching reorganize tests, make devdeps testing optional, unpin latest Python testing, and prepare for possibility of data caching Sep 18, 2023
@zacharyburnett zacharyburnett marked this pull request as ready for review September 18, 2023 18:11
@zacharyburnett zacharyburnett requested a review from a team as a code owner September 18, 2023 18:11
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f77b5d6) 75.42% compared to head (4f7f614) 75.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7910   +/-   ##
=======================================
  Coverage   75.42%   75.42%           
=======================================
  Files         464      464           
  Lines       37938    37938           
=======================================
  Hits        28615    28615           
  Misses       9323     9323           
Flag Coverage Δ *Carryforward flag
nightly 77.37% <ø> (ø) Carriedforward from 4be1691

*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.

@zacharyburnett zacharyburnett force-pushed the ci/separate_devdeps branch 2 times, most recently from 9710a81 to fa1359b Compare September 21, 2023 13:28
@zacharyburnett zacharyburnett force-pushed the ci/separate_devdeps branch 8 times, most recently from d4a7134 to 65a4933 Compare November 17, 2023 12:53
@zacharyburnett zacharyburnett force-pushed the ci/separate_devdeps branch 2 times, most recently from f94669b to 07d5c9d Compare December 1, 2023 16:54
@hbushouse hbushouse added this to the Build 10.1 milestone Dec 1, 2023
@hbushouse
Copy link
Collaborator

Why all the CI failures? Looks like something to do with our pinning of the python version <3.12.

@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Dec 1, 2023

in the regular non-devdeps tests it looks like an issue with timecoeff (I'm not sure what that comes from)
https://github.com/spacetelescope/jwst/actions/runs/7062856370/job/19227602452#step:10:410

=========================== short test summary info ============================
FAILED jwst/photom/tests/test_photom.py::test_miri_image - AttributeError: No attribute 'timecoeff'
= 1 failed, 2705 passed, 532 skipped, 4 xfailed, 110 warnings in 952.53s (0:15:52) =

In the py311-devdeps tests (with 3rd party dev dependencies) the error is with using Numpy 1.x at build time and Numpy 2.x at run time; I'll look into how to resolve that I think we'll just have to wait for a Numpy 2.0 release to resolve this
https://github.com/spacetelescope/jwst/actions/runs/7062856371/job/19227604346#step:10:275

A module that was compiled using NumPy 1.x cannot be run in
NumPy 2.0.0.dev0 as it may crash. To support both 1.x and 2.x
versions of NumPy, modules must be compiled against NumPy 2.0.

Then for the latest Python ones (py3) the error you mentioned:
https://github.com/spacetelescope/jwst/actions/runs/7062856371/job/19227604975#step:10:28

ERROR: Package 'jwst' requires a different Python: 3.12.0 not in '<3.12,>=3.9'

@braingram
Copy link
Collaborator

braingram commented Dec 1, 2023

Re the numpy build vs runtime:
If it's useful, I made a brief attempt to fix this by installing the pyerfa nightly but the run still failed due to drizzle (which might also need to be built against numpy 2.0): #8094

It looks like stdatamodels has the same devdeps issues :-/

@hbushouse
Copy link
Collaborator

Ah yes, the "timecoeff" errors are due to an update in the jwst master that is not yet available in the latest stdatamodels release, which is used by the CI tests. It is available, however, in stdatamodels/master. I guess we need an stdatamodels release ASAP.

@zacharyburnett
Copy link
Collaborator Author

If the regular tests pass here, we can likely merge this and then deal with the Numpy 2.0 devdeps issues in another PR

@hbushouse
Copy link
Collaborator

So the remaining CI errors all seem to be due to numpy.core.multiarray import errors. Is that what you expect and hence is this ready to merge as-is?

@zacharyburnett
Copy link
Collaborator Author

So the remaining CI errors all seem to be due to numpy.core.multiarray import errors. Is that what you expect and hence is this ready to merge as-is?

yes, I think that's expected. This is ready to merge, and then we can fix the devdeps

@hbushouse hbushouse merged commit c5d0144 into spacetelescope:master Dec 6, 2023
33 of 39 checks passed
@zacharyburnett zacharyburnett deleted the ci/separate_devdeps branch December 6, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants