switch from rhdf5 to hdf5r

rcannood commented 3 months ago

Continuation of #166 but from an internal branch.

After some minor improvements, this implementation seems to be passing a few more tests in comparison to the rhdf5 implementation in main.

rcannood commented 2 months ago

Since more tests are succeeding, I'm merging this branch :)

lazappi commented 2 months ago

Does this mean we are officially moving from {rhdf5} to {hdf5r} (and from Bioconductor to CRAN)? I thought that wasn't necessary any more after I got everything working with {rhdf5}? Or is there something else I'm missing?

rcannood commented 2 months ago

I did merge the PR because the hdf5r implementation is working better atm, though I don't know which of the two will work best in the long run.

I'd been considering simply creating two backends (an HDF5RAnnData and an RHDF5AnnData, if you will) to see with which framework achieving full anndata compatibility is the easiest. Without a direct comparison, deciding between rhdf5 and hdf5r seems like a judgement call.

hdf5r:

Supports HDF5 1.8 - 1.12
Native support for boolean types
CRAN is easier to submit to and easier to install packages from

rhdf5:

HDF5 1.10
More frequent commits (that is, better maintained?)

both:

We could temporarily create both backends (hdf5r and rhdf5) so we can directly compare how well each framework works
Criteria
- Fewest compatibility issues
- Read / write speed
- Which versions of hdf5 are supported
- How well maintained the project is

wdyt?

lazappi commented 2 months ago

I guess if they are both implemented already I wouldn't be entirely opposed to that in a testing phase but I definitely wouldn't release both of them. It does seem like it might be a lot of work though, depending on where things are at.

Unless there is a significant issue with one implementation then I think CRAN vs Bioconductor is probably a bigger question than which HDF5 implementation to use. This probably has more to do with the community we want to interact with/reach than anything technical. We did discuss this when we started the project at the hackathon but maybe we need to do it again.

scverse / anndataR

switch from rhdf5 to hdf5r #169