Fix #2812: Drop HDF5 support#3138
Conversation
|
@benjeffery please give it a look; make sure that I did not leave unnecessary tests (that do not fail) |
|
@benjeffery I also updated the license year in some files to 2025 (maybe it should have been a separate PR?) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3138 +/- ##
==========================================
- Coverage 89.72% 89.58% -0.14%
==========================================
Files 29 28 -1
Lines 32225 31841 -384
Branches 5890 5849 -41
==========================================
- Hits 28913 28524 -389
- Misses 1882 1887 +5
Partials 1430 1430
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
benjeffery
left a comment
benjeffery
left a comment
There was a problem hiding this comment.
Looks good!
Can we make the changelog line a bit more descriptive to include a mention of "legacy formats from msprime version blah"
The metadata tests shouldn't be removed, just remove the HDF5 from the name.
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
Looks good. We should do a search for h5py across the full repo to make sure we've cleared it out from all the requirements files etc as well.
| "may have been generated by msprime < 0.6.0 (June 2018) which " | ||
| "can no longer be read directly. Please convert to the new " | ||
| "kastore format using the ``tskit upgrade`` command." | ||
| "kastore format using the ``tskit upgrade`` command from tskit version 0.6.2" |
There was a problem hiding this comment.
Have we removed the "upgrade" CLI then also? I'm not sure what this would do now
There was a problem hiding this comment.
Actually, yes. Now tskit upgrade is totally removed
| the original schema instead (:user:`benjeffery`, :issue:`3129`, :pr:`3130`) | ||
|
|
||
| **Breaking Changes** | ||
| - HDF5 support is dropped (:user:`hossam26644`, :issue:`2812`, :pr:`3138`) |
There was a problem hiding this comment.
Something a bit more descriptive might be helpful here.
|
@benjeffery Ok, I put back the metadataroundtrip testing. It was not converting to and from HDF5 using the tskit apis; was it actually doing any conversions? |
No, the name was a relic from when the file format was HDF5 only. |
hossam26644
force-pushed
the
remove-legacy-h5py
branch
from
2093ebd to
425bc02
Compare
|
@benjeffery ready for another review round |
|
Almost there, we need to remove h5py from all requirements files. |
|
Nice, shedding a bunch of baggage here! |
|
Great! Just a squash now and we should be good to go. |
benjeffery
force-pushed
the
remove-legacy-h5py
branch
from
ac6ddfb to
9bd5c64
Compare
benjeffery
force-pushed
the
remove-legacy-h5py
branch
from
9bd5c64 to
6f17d8c
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
Woop, first PR merged with the new system. This new system gives us a rebase button on PRs too so no need to message to trigger that anymore. |
|
Also getting rid of HDF5 - hooray. |
|
🎉 🎊 |
4 participants
No description provided.