Skip to content

Conversation

@penaguerrero
Copy link
Contributor

@penaguerrero penaguerrero commented Apr 26, 2025

Resolves JP-3988

This PR addresses enhancing the reports for table differences and enables providing strings and integers to the list of HDUs to ignore. The name of this option is ignore_hdus and it lives in the fitsdiff_default_kwargs dictionary set up in the specific test(s).

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

@penaguerrero penaguerrero requested a review from a team as a code owner April 26, 2025 04:17
@codecov
Copy link

codecov bot commented Apr 26, 2025

Codecov Report

❌ Patch coverage is 3.10559% with 156 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.27%. Comparing base (a1cbc2c) to head (4ecbbf2).
⚠️ Report is 694 commits behind head on main.

Files with missing lines Patch % Lines
jwst/regtest/st_fitsdiff.py 1.55% 127 Missing ⚠️
jwst/scripts/stfitsdiff.py 9.37% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9413      +/-   ##
==========================================
+ Coverage   76.58%   77.27%   +0.68%     
==========================================
  Files         364      362       -2     
  Lines       36595    36279     -316     
==========================================
+ Hits        28028    28035       +7     
+ Misses       8567     8244     -323     

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

@melanieclarke melanieclarke added this to the Build 12.0 milestone Apr 28, 2025
@penaguerrero penaguerrero requested a review from a team as a code owner April 29, 2025 14:10
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.

I love the new reports!

There are a couple little weird things to track down in the new table reports and HDU ignoring - I'll keep working with you offline on those.

In the meantime, a couple other small suggestions below, and also:
I can't add a comment in the right place, but for some data I get an annoying RuntimeWarning ("invalid value encountered in subtract") in the diff_total calculation in 982. Can you please filter it out?

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.

All my comments are addressed and local tests look good. No need for regtests, since this code is as-yet unused.

@melanieclarke melanieclarke merged commit cd99999 into spacetelescope:main May 5, 2025
22 of 23 checks passed
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.

2 participants