JP-3768: Move outlier detection median computers to stcal

[emolter]

Resolves JP-3768

Closes #8838

This PR ports the median calculation machinery into stcal for use by other missions. See spacetelescope/stcal#292 for the related changes in stcal.

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
• Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
  • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • start a regression test and include a link to the running job (click here for instructions)
    • Do truth files need to be updated ("okified")?
      • after the reviewer has approved these changes, run okify_regtests to update the truth files
• if a JIRA ticket exists, make sure it is resolved properly

news fragment change types...

• changes/<PR#>.general.rst: infrastructure or miscellaneous change
• changes/<PR#>.docs.rst
• changes/<PR#>.stpipe.rst
• changes/<PR#>.datamodels.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.fits_generator.rst
• changes/<PR#>.set_telescope_pointing.rst
• changes/<PR#>.pipeline.rst

stage 1

• changes/<PR#>.group_scale.rst
• changes/<PR#>.dq_init.rst
• changes/<PR#>.emicorr.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.ipc.rst
• changes/<PR#>.firstframe.rst
• changes/<PR#>.lastframe.rst
• changes/<PR#>.reset.rst
• changes/<PR#>.superbias.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.rscd.rst
• changes/<PR#>.persistence.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.charge_migration.rst
• changes/<PR#>.jump.rst
• changes/<PR#>.clean_flicker_noise.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.gain_scale.rst

stage 2

• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.badpix_selfcal.rst
• changes/<PR#>.msaflagopen.rst
• changes/<PR#>.nsclean.rst
• changes/<PR#>.imprint.rst
• changes/<PR#>.background.rst
• changes/<PR#>.extract_2d.rst
• changes/<PR#>.master_background.rst
• changes/<PR#>.wavecorr.rst
• changes/<PR#>.srctype.rst
• changes/<PR#>.straylight.rst
• changes/<PR#>.wfss_contam.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.fringe.rst
• changes/<PR#>.pathloss.rst
• changes/<PR#>.barshadow.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.pixel_replace.rst
• changes/<PR#>.resample_spec.rst
• changes/<PR#>.residual_fringe.rst
• changes/<PR#>.cube_build.rst
• changes/<PR#>.extract_1d.rst
• changes/<PR#>.resample.rst

stage 3

• changes/<PR#>.assign_mtwcs.rst
• changes/<PR#>.mrs_imatch.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.exp_to_source.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.tso_photometry.rst
• changes/<PR#>.stack_refs.rst
• changes/<PR#>.align_refs.rst
• changes/<PR#>.klip.rst
• changes/<PR#>.spectral_leak.rst
• changes/<PR#>.source_catalog.rst
• changes/<PR#>.combine_1d.rst
• changes/<PR#>.ami.rst

other

• changes/<PR#>.wfs_combine.rst
• changes/<PR#>.white_light.rst
• changes/<PR#>.cube_skymatch.rst
• changes/<PR#>.engdb_tools.rst
• changes/<PR#>.guider_cds.rst

[emolter]

starting regression tests here using the stcal PR branch as a dependency
edit: no failures

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.77%. Comparing base (5868435) to head (4b78ba8).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8840      +/-   ##
==========================================
- Coverage   61.89%   61.77%   -0.12%     
==========================================
  Files         377      377              
  Lines       38954    38835     -119     
==========================================
- Hits        24110    23991     -119     
  Misses      14844    14844              

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

[melanieclarke]

LGTM, pending merge of the stcal PR. I like the new append and evaluate methods much better than the old helper functions!

[braingram]

Somewhat unrelated to this PR (so feel free to say "make a ticket") but doesn't the above line modify the input model data?

[emolter]

I think you mean a different line. How does appending drizzled_model.data to the computer modify it?

I'm a little confused about the "never modify the input data" rule. Does that mean that for a step like outlier detection where you have a ModelLibrary input and output, you should never call modify=True when shelving?

I'd say let's put any changes into a different refactor or PR, unless you have concerns w.r.t. the functions that are going into stcal.

[braingram]

Line 98 is that one that can modify the input data (setting an input pixel to nan even if it's not an outlier). I opened an issue: #8845

The "never modify the input data" rule is certainly beyond the scope of this PR (and doesn't really apply for most if not all of image 3).

No concerns about this PR. I just brought it up since I noticed it while working on the corresponding romancal PR and wanted to check if I was mistaken.

[braingram]

LGTM. It's nice to review a PR with 8 added lines of code :)

Just 1 comment about the non-resampled median about a change from a prior PR. It probably makes sense to handle it in a separate PR (in case creating a data copy does something unexpected).

[emolter]

spacetelescope/stcal#292 is approved and merged. What changes if any do you recommend to pyproject.toml before merging this one? Or do we need to wait for an stcal release? @braingram @melanieclarke

[braingram]

spacetelescope/stcal#292 is approved and merged. What changes if any do you recommend to pyproject.toml before merging this one? Or do we need to wait for an stcal release? @braingram @melanieclarke

Roman folks typically do something like:

#    "stcal>=1.8.0,<1.9.0",
    "stcal @ git+https://github.com/spacetelescope/stcal.git@main",

I think that should work here. We'll of course need a release of stcal before any release of jwst.

[emolter]

started a (hopefully final) round of regression tests to ensure the correct stcal version gets picked up and works properly

[braingram]

started a (hopefully final) round of regression tests to ensure the correct stcal version gets picked up and works properly

The jenkins runs are a bit of a lie for changes like this. They always use dev requirements for some packages:

jwst/JenkinsfileRT

Line 76 in 7248358

bc0.pip_reqs_files = ['requirements-dev-st.txt', 'requirements-sdp.txt']

one of which is stcal:

jwst/requirements-dev-st.txt

Line 5 in 7248358

git+https://github.com/spacetelescope/stcal

So anything listed in pyproject.toml for stcal is ignored for jenkins.

[emolter]

The jenkins runs are a bit of a lie for changes like this. They always use dev requirements for some packages:

Oh, right, I think I already knew that. Nevertheless, the tests should pass now that the merge into stcal happened, right?

[braingram]

Oh, right, I think I already knew that. Nevertheless, the tests should pass now that the merge into stcal happened, right?

They should. Also, I don't believe the github actions regtests by default install requirements-dev-st.txt (which I think makes sense but I'm happy to discuss this with anyone interested) so the update to pyproject.toml should also:

• fix the github CI for this PR (and subsequent PRs once this is merged)
• fix the github regression tests
• fix installs for users/developers who pip install from the dev branch
• provide a more standard description of the actual requirements