Skip to content
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

OrderedCollections, an implicit dependency #1046

Closed
vadim-z opened this issue Jan 31, 2023 · 4 comments · Fixed by #1047
Closed

OrderedCollections, an implicit dependency #1046

vadim-z opened this issue Jan 31, 2023 · 4 comments · Fixed by #1047

Comments

@vadim-z
Copy link

vadim-z commented Jan 31, 2023

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
Copy link
Member

musm commented Feb 1, 2023

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
Copy link
Member

mkitti commented Feb 1, 2023

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

@simonbyrne
Copy link
Collaborator

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:

HDF5.jl/src/fileio.jl

Lines 15 to 16 in 9a90e4e

_infer_track_order(track_order::Union{Nothing,Bool}, dict::AbstractDict) =
track_order === nothing ? isa(dict, OrderedDict) : track_order

I think we can work around this

@musm
Copy link
Member

musm commented Feb 1, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants