Skip to content

Add read pattern test.#2390

Draft
schlafly wants to merge 2 commits into
spacetelescope:mainfrom
schlafly:add-read-pattern-test
Draft

Add read pattern test.#2390
schlafly wants to merge 2 commits into
spacetelescope:mainfrom
schlafly:add-read-pattern-test

Conversation

@schlafly

@schlafly schlafly commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

This PR adds a new test to make sure that read_pattern is propagated into catalogs / segmentation maps. It is a nested arrays and so there is some roman_datamodels handling that can cause problems.

Only adds a unit test (which should fail). Regtests pointing to spacetelescope/roman_datamodels#692 should pass:
https://github.com/spacetelescope/RegressionTests/actions/runs/29610396350

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 changelog readme for instructions)
      • if your change breaks existing functionality, also add a changes/<PR#>.breaking.rst news fragment
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
  • if a JIRA ticket exists, make sure it is resolved properly

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.82%. Comparing base (4536045) to head (4390de8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2390   +/-   ##
=======================================
  Coverage   82.82%   82.82%           
=======================================
  Files         131      131           
  Lines        7249     7249           
=======================================
  Hits         6004     6004           
  Misses       1245     1245           

☔ View full report in Codecov by Harness.
📢 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.

Remove redundant comment about regression test.
@schlafly

Copy link
Copy Markdown
Collaborator Author

@braingram or @WilliamJamieson , any interest reviewing? This PR just adds a unit test that currently fails; when copying metadata into the source catalog or segmentation map from the L2 image, some array fields do not survive. This test demonstrates that problem. spacetelescope/roman_datamodels#692 intends to fix the issue. That does work; the regression test run
https://github.com/spacetelescope/RegressionTests/actions/runs/29610396350
passes. But I haven't looked carefully at the roman_datamodels code I'm changing.

@braingram

Copy link
Copy Markdown
Collaborator

Should we consider #2237 so we can compare the truth metadata to the regtest results?

@schlafly

Copy link
Copy Markdown
Collaborator Author

Ugh, sorry I lost #2237. Let me respond in the thread there about that one...

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants