Skip to content

Conversation

emolter
Copy link
Collaborator

@emolter emolter commented Mar 18, 2025

Provides test coverage for JP-3686. See that issue's GitHub mirror at #8639

The changes associated with that ticket are in stcal, but a regtest is needed to prevent future bugs like the one discussed in the ticket comments, which is resolved by spacetelescope/stcal#355.

This PR adds a regression test that takes the output _cat.ecsv file directly from a run of the SourceCatalogStep and injects it back into TweakRegStep as the abs_refcat parameter.

I confirmed that the test fails on the current stable release of stcal, but passes with the bugfix in spacetelescope/stcal#355, so it indeed tests what it is intended to test.

This PR also adds better documentation of the abs_refcat and catfile parameters.

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#>.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
Copy link
Collaborator Author

emolter commented Mar 18, 2025

@emolter emolter changed the title JP-3686: Test use source catalog output as tweakreg abs_refcat JP-3686: Test using source catalog output as tweakreg abs_refcat Mar 18, 2025
Copy link

codecov bot commented Mar 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.62%. Comparing base (09fd95a) to head (6e2e607).
Report is 757 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9301      +/-   ##
==========================================
- Coverage   74.69%   74.62%   -0.08%     
==========================================
  Files         369      369              
  Lines       37105    37152      +47     
==========================================
+ Hits        27717    27726       +9     
- Misses       9388     9426      +38     

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

@emolter emolter requested review from drlaw1558 and tapastro March 18, 2025 21:40
@emolter emolter marked this pull request as ready for review March 19, 2025 02:42
@emolter emolter requested review from a team as code owners March 19, 2025 02:42
@drlaw1558
Copy link
Collaborator

Tiny comment; the 'Alignment' section of README.rst now has some duplicated/conflicting information. Paragraph 2 adds the new comment about catalog needing either RA/DEC or sky_centroid, but paragraph 4 still says that it must contain RA/DEC. We should condense these paragraphs.

@emolter
Copy link
Collaborator Author

emolter commented Mar 19, 2025

@drlaw1558 How does 40567f3 look?

Copy link
Contributor

@tapastro tapastro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tapastro tapastro enabled auto-merge (squash) March 26, 2025 14:00
@tapastro tapastro merged commit 5a786cb into spacetelescope:main Mar 26, 2025
25 checks passed
@emolter emolter deleted the JP-3686 branch March 26, 2025 15:14
@tapastro
Copy link
Contributor

@meeseeksdev backport to release/1.18.x

meeseeksmachine pushed a commit to meeseeksmachine/jwst that referenced this pull request Mar 31, 2025
@pllim pllim modified the milestones: Build 12.0, Build 11.3 Mar 31, 2025
tapastro pushed a commit that referenced this pull request Mar 31, 2025
…e catalog output as tweakreg abs_refcat) (#9346)

Co-authored-by: Ned Molter <[email protected]>
tapastro added a commit that referenced this pull request Apr 8, 2025
tapastro added a commit that referenced this pull request Apr 8, 2025
…ng source catalog output as tweakreg abs_refcat)" (#9375)
@pllim pllim modified the milestones: Build 11.3, Build 12.0 Apr 8, 2025
@pllim
Copy link
Collaborator

pllim commented Apr 8, 2025

Given #9375 , I updated the milestone.

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.

4 participants