spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
545 stars 158 forks source link

remove requirements-max.txt #8574

Closed braingram closed 1 week ago

braingram commented 2 weeks ago

The requirements-max.txt file contains 2 upper pins: https://github.com/spacetelescope/jwst/blob/380dc5eb082c6c5d4e024a9337999f69cd5b8443/requirements-max.txt#L3-L4

and is used in the main (non-dev) jenkins file: https://github.com/spacetelescope/jwst/blob/380dc5eb082c6c5d4e024a9337999f69cd5b8443/JenkinsfileRT#L77

This appears to be run after the pip list in the job which may mean that our regtests have been using a version of stcal that is lower than our minimum version listed in pyproject.toml: https://github.com/spacetelescope/jwst/blob/380dc5eb082c6c5d4e024a9337999f69cd5b8443/pyproject.toml#L37

As requirements-max.txt is not used for the GA regression tests (and at the moment only show differences due to different machines) this PR removes the file (and the use in the jenkins regtests).

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.51%. Comparing base (dba65ae) to head (d668926). Report is 4 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8574 +/- ## ========================================== + Coverage 58.63% 59.51% +0.88% ========================================== Files 391 391 Lines 39081 39254 +173 ========================================== + Hits 22914 23362 +448 + Misses 16167 15892 -275 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zacharyburnett commented 2 weeks ago

looks like it was because of an issue with building a test environment, where we couldn't install user-requested dependencies on a Jenkins run if they were constrained by the upper pins in pyproject.toml: https://github.com/spacetelescope/jwst/pull/7575

however, in the new GitHub Actions setup we install all "override" dependencies requested by the user as the last possible step, after the initial install, so this is no longer an issue

braingram commented 2 weeks ago

Regtests running at: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1541/

braingram commented 1 week ago

I made a test branch that prints out the stcal and stdatamodels versions at the start of the pytest run: https://github.com/spacetelescope/jwst/compare/master...braingram:jwst:test_max and triggered a jenkins run (that ran only 1 test as only the header is useful for this test branch): https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%2FJWST-Developers-Pull-Requests/detail/JWST-Developers-Pull-Requests/1542/pipeline/194 The header included:

============================= test session starts ==============================
platform linux -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0
crds_context: jwst_1241.pmap
stcal: 1.7.3.dev6+g3fa4d42
stdatamodels: 1.10.1

Even though the requirements-max.txt file has stdatamodels<1.10.0. Looking at the console output for this run: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1542/consoleFull I see the requirements-max.txt used (which installs an older version of stdatamodels) before pip install -e '.[test,sdp]' --no-cache-dir. (which uninstalls the older version and installs one above the pin in requirements-max.txt).

At this point I'm pretty confused about what conditions are required to have this requirements-max.txt have an effect (is it used for the non-PR job? is it used only for certain triggers of the job?).

zacharyburnett commented 1 week ago

At this point I'm pretty confused about what conditions are required to have this requirements-max.txt have an effect (is it used for the non-PR job? is it used only for certain triggers of the job?).

now I'm unsure if it ever worked at all, or was just a placebo PR... I don't see any reason to keep it