-
Notifications
You must be signed in to change notification settings - Fork 177
JP-3908: Incorporate stfitsdiff into regression tests #9414
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.
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9414 +/- ##
==========================================
- Coverage 77.93% 77.85% -0.08%
==========================================
Files 362 362
Lines 36252 36288 +36
==========================================
Hits 28253 28253
- Misses 7999 8035 +36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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. I don't see any reason why we shouldn't just update all of these at once as you suggest, IMO the new version is strictly better (or will be once any remaining kinks are ironed out).
…to improve reports.
…to improve reports.
…to improve reports.
@penaguerrero - now that the table diffs are merged, can you please run regression tests again? Then I think we should be okay to get this merged too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like regtests caught a problem with the table reports we overlooked in the last PR, for non-float data. I'm going to mark this as changes-requested until we get that fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes! I think we're ready to give this a try.
Resolves JP-3908
This PR implements STFITSDiff instead of astropy.FITSDiff to show our ad hoc reports.
Tasks
Build 12.0
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see changelog readme for instructions)docs/
pageokify_regtests
to update the truth files