search

JuliaIO / HDF5.jl

Save and load data in the HDF5 file format from Julia
https://juliaio.github.io/HDF5.jl
MIT License
386 stars 140 forks source link

OrderedCollections, an implicit dependency #1046

Closed vadim-z closed 1 year ago

vadim-z commented 1 year ago

First, thank you for this important package.

I'd like to use FileIO load function with HDF5 file. I ]add and import HDF5 and FileIO packages.

Because of the following lines I need to add and also import (due to lazy loading) OrderedCollections package.

Unless this is done, I receive somewhat cryptic error messages:

Error encountered while load File{DataFormat{:HDF5}, String}("gen1.h5").

Fatal error: ERROR: HDF5 load error: neither load nor fileio_load is defined due to FileIO.SpecError(HDF5, :load) Will try next loader.

which give no hints about OrderedCollections. After this package is added and imported, the error goes away.

Since neither HDF5.jl nor FileIO.jl depend on OrderedCollections and, on the other hand, the package involved is rather small and has no dependencies itself, I suggest making it an explicit and unconditional dependency of HDF5.jl. Or at least please warn users about the problem.

With best regards, Vadim Zborovskii

musm commented 1 year ago

Interesting, so how again does this crop up? Any more information on the setup or a reproducible example? I am not against unconditionally requiring OrderedCollections, but it would be interesting to see why those lines fail. If you look at the manifest we do have it: https://github.com/JuliaIO/HDF5.jl/blob/master/Project.toml#L30 but it's not a required dep.

mkitti commented 1 year ago

We probably want to wait until glue packages in Julia 1.9 come around to really fix this.

simonbyrne commented 1 year ago

I think the issue is that the code is included only if both FileIO and OrderedCollections are loaded, simply having FileIO only is insufficient. It is only required because of this line: https://github.com/JuliaIO/HDF5.jl/blob/9a90e4e5580878b64be2cb70799bf27221f11150/src/fileio.jl#L15-L16

I think we can work around this

musm commented 1 year ago

Oh I see it, right, if you want to use FileIO and HDF5 together, since HDF5 needs OrderedCollections the end user needs to import it first or that fileio.jl file won't work.