8000 JP-3116: Do not create observation-only Level 2 associations if part of a background candidate by tapastro · Pull Request #9098 · spacetelescope/jwst · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

JP-3116: Do not create observation-only Level 2 associations if part of a background candidate #9098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Feb 4, 2025
Merged

JP-3116: Do not create observation-only Level 2 associations if part of a background candidate #9098

merged 6 commits into from
Feb 4, 2025

Conversation

tapastro
Copy link
Contributor

Resolves JP-3116

Closes #7819

This PR adds conditional behavior for association generation for DMS; when generating associations for a observation candidate ("o-type"), any science exposures belonging to a background ("c-type") candidate are dropped from consideration. This PR propagates the DMS flag of asn_generate down to the appropriate function where the new functionality was added.

Currently in draft; this PR as currently written also modifies level 3 association behavior, which is not strictly part of the JIRA ticket request.

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

@tapastro tapastro requested a review from a team as a code owner January 27, 2025 22:33
@tapastro tapastro marked this pull request as draft January 27, 2025 22:33
@codecov Codecov
Copy link
codecov bot commented Jan 27, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.74%. Comparing base (1ac0403) to head (dba92e1).
Report is 869 commits behind head on main.

Files with missing lines Patch % Lines
...t/associations/generator/generate_per_candidate.py 87.50% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #9098      +/-   ##
==========================================
- Coverage   77.64%   73.74%   -3.91%     
==========================================
  Files         509      372     -137     
  Lines       46794    37274    -9520     
==========================================
- Hits        36332    27486    -8846     
+ Misses      10462     9788     -674
Flag Coverage Δ
nightly ?

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

@tapastro tapastro force-pushed the jp-3116-bkgd-ctype-asns branch from 6330761 to 64cde4d Compare January 30, 2025 20:46
@tapastro
Copy link
Contributor Author

Regtests including new coverage started here: https://github.com/spacetelescope/RegressionTests/actions/runs/13061128511

@tapastro tapastro marked this pull request as ready for review February 3, 2025 14:30
@tapastro tapastro requested a review from a team as a code owner February 3, 2025 14:30
@tapastro tapastro mentioned this pull request Feb 3, 2025
Merged
10 tasks
@tapastro
Copy link
Contributor Author
tapastro commented Feb 3, 2025

Failures on tests were due to a MIRI MRS CRDS delivery which have since been okified.

melanieclarke
melanieclarke reviewed Feb 3, 2025
View reviewed changes
Copy link
Collaborator
@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

This looks fine from a software perspective. I'll defer approval until the association experts review.

@tapastro tapastro force-pushed the jp-3116-bkgd-ctype-asns branch from ee7a6b2 to 1c31d2e Compare February 3, 2025 21:31
@melanieclarke
Copy link
Collaborator

LGTM, thanks. Do you want to wait for Jonathan to review?

@tapastro
Copy link
Contributor Author
tapastro commented Feb 4, 2025

Starting another set of tests: https://github.com/spacetelescope/RegressionTests/actions/runs/13139765289

@tapastro
Copy link
Contributor Author
tapastro commented Feb 4, 2025

LGTM, thanks. Do you want to wait for Jonathan to review?

I don't know that he'll find time for it, and we've talked about the change. I think it's good to go.

melanieclarke
melanieclarke approved these changes Feb 4, 2025
View reviewed changes
Copy link
Collaborator
@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

Approved, pending regtests!

@tapastro
Copy link
Contributor Author
tapastro commented Feb 4, 2025

Looks like it's just the known guider failures and an EngDB connection issue in the failures - merging now.

@tapastro tapastro merged commit 0984688 into spacetelescope:main Feb 4, 2025
29 of 30 checks passed
@tapastro tapastro deleted the jp-3116-bkgd-ctype-asns branch February 4, 2025 17:54
@stscijgbot-jp stscijgbot-jp mentioned this pull request Feb 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melanieclarke melanieclarke melanieclarke approved these changes

@stscieisenhamer stscieisenhamer Awaiting requested review from stscieisenhamer

Assignees
No one assigned
Labels
associations regression_testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not create observation-only Level 2 associations if part of a background candidate
2 participants
@tapastro @melanieclarke
0