Skip to content

Conversation

@emolter
Copy link
Collaborator

@emolter emolter commented Jul 28, 2025

Required before spacetelescope/stdatamodels#539 can be merged

This PR updates the unit tests of exp_to_source to take in mock data instead of loading fits files from tests/data/. Mocking the data is a better pattern for unit tests.

This is required to move forward with the removal of the unused V23ToSky transform model, which was replaced by Astropy standard models about 5 years ago (#5206), because the test files are old enough that their WCS still relies on V23ToSky.

There is still open discussion about whether that removal should happen, but these test updates are a good idea in any case.

This PR only touches tests, so no changelog is warranted.

Tasks

  • If you have a specific reviewer in mind, tag them.
  • add a build milestone, i.e. Build 12.0 (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)
    • 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

@emolter
Copy link
Collaborator Author

emolter commented Jul 28, 2025

@emolter emolter changed the title Replace real test data with mock data for exp_to_source unit tests Update exp_to_source unit tests to use mock data instead of loading from file Jul 28, 2025
@codecov
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.26%. Comparing base (0a6e722) to head (10e481c).
⚠️ Report is 287 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9706      +/-   ##
==========================================
- Coverage   81.28%   81.26%   -0.02%     
==========================================
  Files         368      368              
  Lines       37501    37501              
==========================================
- Hits        30484    30477       -7     
- Misses       7017     7024       +7     

☔ 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 marked this pull request as ready for review July 28, 2025 16:51
@emolter emolter requested a review from a team as a code owner July 28, 2025 16:51
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.

LGTM, thanks for the clean up.

@melanieclarke melanieclarke merged commit 99d88fa into spacetelescope:main Jul 29, 2025
32 checks passed
@emolter emolter deleted the exp-to-source-test branch July 29, 2025 18:10
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.

3 participants