search

spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io
Other
5 stars 25 forks source link

JP-3656: Removed unused DrizPars schemas and models #316

Closed emolter closed 1 month ago

emolter commented 2 months ago

Resolves JP-3656

Closes JWST 8558 Closes #304

This PR removes the unused DrizPars model type for JWST and the associated R_DRIZPAR keyword in the core schema, as well as associated imports and docs.

Checklist

codecov[bot] commented 2 months ago

Codecov Report

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

Project coverage is 66.18%. Comparing base (9376300) to head (84279cf). Report is 10 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #316 +/- ## ========================================== - Coverage 66.21% 66.18% -0.04% ========================================== Files 101 100 -1 Lines 5402 5397 -5 ========================================== - Hits 3577 3572 -5 Misses 1825 1825 ```

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

emolter commented 2 months ago

Four failing tests here. One matches the nightly test runs; the other three appear to be related to a necessary CRDS update. Can someone please advise me on where those come from and how they should be fixed? I would be happy to make a PR in CRDS (if that's indeed what's required), but I've never touched that code before and could use some guidance.

braingram commented 2 months ago

The drizpars ones are a result of the tests that this repository contains that check that "all" reference files from crds can be loaded. Those failures are expected since drizpars support is removed with this PR.

I would be fine with disabling the tests against the drizpars reference files. @tapastro any objection?

The other failure on main is unrelated. Presumably it's from a crds update.

braingram commented 2 months ago

If ignoring drizpars is acceptable, it can be skipped somewhere here: https://github.com/spacetelescope/stdatamodels/blob/ee4756ffe9ff71c1fb7b067f8998949a88d6a703/src/stdatamodels/jwst/datamodels/tests/test_schema_against_crds.py#L194-L200 (similar to how the pars- files are skipped).

braingram commented 2 months ago

Would you run regtests with this PR? I'm assuming it will show many expected failures due to the missing R_DRIZPAR.

emolter commented 2 months ago

regression tests for JWST started here

edit: sure enough, there are 55 failures, although the MIRI LRS slitless ones are unrelated

braingram commented 1 month ago

I'll leave merging up to you (let me know if you don't have permission) to schedule the regtest okifying. Thanks!

emolter commented 1 month ago

@tapastro are the JWST regression tests in a good enough state for us to merge this and for me to ok-ify as I see fit afterward?

emolter commented 1 month ago

regression tests to okify are started here