Add test for duplicate data when resaving model to fits #7358
Conversation
Codecov ReportBase: 79.63% // Head: 79.76% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #7358 +/- ##
==========================================
+ Coverage 79.63% 79.76% +0.12%
==========================================
Files 412 412
Lines 37572 37572
==========================================
+ Hits 29922 29970 +48
+ Misses 7650 7602 -48
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
braingram
force-pushed
the
bug/fits_data_duplication
branch
from
33dd324
to
082423a
Compare
|
Thanks! I made the improvements your described. Let me know if there's anything else and if you think this should either be merged now (with the test skipped) or wait until there's a fix for the bug in asdf. Either way I'll mark it ready for review. |
|
Shall we go ahead and merge this now, because it's being skipped and hence can't cause any damage, or wait for fix in asdf to be released (which could be a while)? |
braingram
force-pushed
the
bug/fits_data_duplication
branch
from
082423a
to
818ea50
Compare
When resaving a model to fits, asdf loses track of array-hdu links and saves arrays in both hdu and as internal blocks in the asdf tree. This can make an asdf tree that is larger than what can be stored in a fits hdu. See asdf issue: asdf-format/asdf#1232 This test is skipped as it fails with the current asdf version. modify imports and use context manager for test_fits Co-authored-by: James Davies <jdavies@mpia.de> use context manager in test_fits Co-authored-by: James Davies <jdavies@mpia.de> improve test docstring in test_fits
unskip test for issue spacetelescope#7354 fixes spacetelescope#7354
braingram
force-pushed
the
bug/fits_data_duplication
branch
from
818ea50
to
a551775
Compare
|
I removed the skip and bumped the asdf version to 2.14.1 to include the fix added in asdf 2.14. For reference here's the relevant PR to asdf with the fix: |
|
The CI test-xdist-cov errors appear to be unrelated and possibly addressed in this PR: #7370 |
|
Agreed, the failure is unrelated. Merging this so we can rerun the failing OPS case. |
When resaving a model to fits, asdf loses track of array-hdu links and saves arrays in both hdu and as internal blocks in the asdf tree. This can make an asdf tree that is larger than what can be stored in a fits hdu.
See asdf issue: asdf-format/asdf#1232
This test is skipped as it fails with the current asdf version.
Checklist for maintainers
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR