Skip to content

Conversation

@braingram
Copy link
Collaborator

@braingram braingram commented Apr 2, 2025

Update SourceCatalogStep to replace uses of maker_utils with non-test code.

Regtests all pass: https://github.com/spacetelescope/RegressionTests/actions/runs/14223844319

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. 24Q4_B15 (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#>.associations.rst
  • changes/<PR#>.scripts.rst
  • changes/<PR#>.mosaic_pipeline.rst
  • changes/<PR#>.patch_match.rst

steps

  • changes/<PR#>.dq_init.rst
  • changes/<PR#>.saturation.rst
  • changes/<PR#>.refpix.rst
  • changes/<PR#>.linearity.rst
  • changes/<PR#>.dark_current.rst
  • changes/<PR#>.jump_detection.rst
  • changes/<PR#>.ramp_fitting.rst
  • changes/<PR#>.assign_wcs.rst
  • changes/<PR#>.flatfield.rst
  • changes/<PR#>.photom.rst
  • changes/<PR#>.flux.rst
  • changes/<PR#>.source_detection.rst
  • changes/<PR#>.tweakreg.rst
  • changes/<PR#>.skymatch.rst
  • changes/<PR#>.outlier_detection.rst
  • changes/<PR#>.resample.rst
  • changes/<PR#>.source_catalog.rst

@codecov
Copy link

codecov bot commented Apr 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.98%. Comparing base (327f3f9) to head (00cdc1d).
Report is 152 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1675      +/-   ##
==========================================
+ Coverage   79.95%   79.98%   +0.03%     
==========================================
  Files         116      116              
  Lines        6564     6575      +11     
==========================================
+ Hits         5248     5259      +11     
  Misses       1316     1316              

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

@braingram braingram marked this pull request as ready for review April 2, 2025 16:24
@braingram braingram requested review from a team as code owners April 2, 2025 16:24
Copy link

@perrygreenfield perrygreenfield left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@nden nden left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

Looks good to me. We should talk about whether we need a less intimidating version of
source_catalog_model.meta._schema_attributes.explicit_properties
This kind of "pass through the metadata of the caller" seems like a case we might want to support.

@schlafly schlafly merged commit dad2270 into spacetelescope:main Apr 2, 2025
31 checks passed
@braingram braingram deleted the source_catalog_no_maker_utils branch April 2, 2025 20:10
@perrygreenfield
Copy link

We should talk about whether we need a less intimidating version of
source_catalog_model.meta._schema_attributes.explicit_properties

Hey, that was my favorite change. Actually, I did comment on that offline as a very mysterious line of code. Python black magic at work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants