Skip to content

Update resample step WCS reproject after JP-3824 bugfix in stcal#9023

Closed
emolter wants to merge 3 commits into
spacetelescope:mainfrom
emolter:mihai-bbox
Closed

Update resample step WCS reproject after JP-3824 bugfix in stcal#9023
emolter wants to merge 3 commits into
spacetelescope:mainfrom
emolter:mihai-bbox

Conversation

@emolter

@emolter emolter commented Dec 20, 2024

Copy link
Copy Markdown
Collaborator

With spacetelescope/stcal#326, the lines of code from #8554 that set bounding_box=None when computing the backward transform inside resample_utils.reproject() are no longer necessary. Those code lines bypassed a bug where bounding_box did not respect a nonzero crpix, and that bug is fixed by the aforementioned stcal changes.

This PR doesn't change any actual results, but it makes it so that bad WCSs and/or bounding boxes are not "hidden", and it also avoids deepcopying a WCS which can be computationally intensive.

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

emolter commented Dec 20, 2024

Copy link
Copy Markdown
Collaborator Author

regression tests started here, setting the stcal version to Mihai's corresponding stcal PR

https://github.com/spacetelescope/RegressionTests/actions/runs/12434172392

@emolter
emolter requested review from mcara and nden December 20, 2024 19:57
@emolter emolter changed the title Revert manual setting of bbox=None in resample step WCS reproject after JP-3824 bugfix in stcal Update resample step WCS reproject after JP-3824 bugfix in stcal Dec 20, 2024
@codecov

codecov Bot commented Dec 20, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.83%. Comparing base (095d364) to head (6c68e61).
Report is 1222 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9023      +/-   ##
==========================================
- Coverage   76.89%   76.83%   -0.06%     
==========================================
  Files         497      497              
  Lines       45656    45660       +4     
==========================================
- Hits        35108    35085      -23     
- Misses      10548    10575      +27     
Flag Coverage Δ *Carryforward flag
nightly 77.40% <ø> (+<0.01%) ⬆️ Carriedforward from e3d263f

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@melanieclarke

Copy link
Copy Markdown
Collaborator

@emolter - is this PR still relevant?

@emolter

emolter commented Feb 24, 2025

Copy link
Copy Markdown
Collaborator Author

I think yes, the changes are still worth making. Pinging @mcara here to get his opinion though

This code block got moved to stcal, let me see if it's already fixed there.

No, this is no longer relevant, the code is moved to stcal, and fixed

@emolter emolter closed this Feb 24, 2025
@emolter
emolter deleted the mihai-bbox branch February 24, 2025 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants