Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/api/functions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5522,7 +5522,6 @@ See `libhdf5` documentation for [`H5Pset_obj_track_times`](https://support.hdfgr
"""
function h5p_set_obj_track_times(plist_id, track_times)
lock(liblock)
@show :h5p_set_obj_track_times track_times

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably unrelated.

var"#status#" = try
ccall((:H5Pset_obj_track_times, libhdf5), herr_t, (hid_t, UInt8), plist_id, track_times)
finally
Expand Down
23 changes: 12 additions & 11 deletions src/api/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -523,17 +523,18 @@ const H5T_C_S1 = _read_const(:H5T_C_S1_g)
const H5T_STD_REF_OBJ = _read_const(:H5T_STD_REF_OBJ_g)
const H5T_STD_REF_DSETREG = _read_const(:H5T_STD_REF_DSETREG_g)
# Native types
const H5T_NATIVE_B8 = _read_const(:H5T_NATIVE_B8_g)
const H5T_NATIVE_INT8 = _read_const(:H5T_NATIVE_INT8_g)
const H5T_NATIVE_UINT8 = _read_const(:H5T_NATIVE_UINT8_g)
const H5T_NATIVE_INT16 = _read_const(:H5T_NATIVE_INT16_g)
const H5T_NATIVE_UINT16 = _read_const(:H5T_NATIVE_UINT16_g)
const H5T_NATIVE_INT32 = _read_const(:H5T_NATIVE_INT32_g)
const H5T_NATIVE_UINT32 = _read_const(:H5T_NATIVE_UINT32_g)
const H5T_NATIVE_INT64 = _read_const(:H5T_NATIVE_INT64_g)
const H5T_NATIVE_UINT64 = _read_const(:H5T_NATIVE_UINT64_g)
const H5T_NATIVE_FLOAT = _read_const(:H5T_NATIVE_FLOAT_g)
const H5T_NATIVE_DOUBLE = _read_const(:H5T_NATIVE_DOUBLE_g)
const H5T_NATIVE_B8 = _read_const(:H5T_NATIVE_B8_g)
const H5T_NATIVE_INT8 = _read_const(:H5T_NATIVE_INT8_g)
const H5T_NATIVE_UINT8 = _read_const(:H5T_NATIVE_UINT8_g)
const H5T_NATIVE_INT16 = _read_const(:H5T_NATIVE_INT16_g)
const H5T_NATIVE_UINT16 = _read_const(:H5T_NATIVE_UINT16_g)
const H5T_NATIVE_INT32 = _read_const(:H5T_NATIVE_INT32_g)
const H5T_NATIVE_UINT32 = _read_const(:H5T_NATIVE_UINT32_g)
const H5T_NATIVE_INT64 = _read_const(:H5T_NATIVE_INT64_g)
const H5T_NATIVE_UINT64 = _read_const(:H5T_NATIVE_UINT64_g)
const H5T_NATIVE_FLOAT16 = _read_const(:H5T_NATIVE_FLOAT16_g)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need something for earlier versions of HDF5 here (pre 2.0).

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H5T_NATIVE_FLOAT16 is loaded unconditionally via _read_const(:H5T_NATIVE_FLOAT16_g). Since _read_const uses dlsym(...; throw_error=true) this will raise during module load if the linked libhdf5 (or the shim exporting *_g symbols) doesn’t provide that symbol. Please gate this with _has_symbol(:H5T_NATIVE_FLOAT16_g) (or a libversion check) and either provide a fallback type id or throw a clear runtime error when Float16 writes are attempted on unsupported HDF5 builds.

Suggested change
const H5T_NATIVE_FLOAT16 = _read_const(:H5T_NATIVE_FLOAT16_g)
const H5T_NATIVE_FLOAT16 = _has_symbol(:H5T_NATIVE_FLOAT16_g) ? _read_const(:H5T_NATIVE_FLOAT16_g) : hid_t(-1)

Copilot uses AI. Check for mistakes.
const H5T_NATIVE_FLOAT = _read_const(:H5T_NATIVE_FLOAT_g)
const H5T_NATIVE_DOUBLE = _read_const(:H5T_NATIVE_DOUBLE_g)
# Other type constants
const H5T_VARIABLE = reinterpret(UInt, -1)

Expand Down
5 changes: 4 additions & 1 deletion src/typeconversions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ hdf5_type_id(::Type{Int32}) = API.H5T_NATIVE_INT32
hdf5_type_id(::Type{UInt32}) = API.H5T_NATIVE_UINT32
hdf5_type_id(::Type{Int64}) = API.H5T_NATIVE_INT64
hdf5_type_id(::Type{UInt64}) = API.H5T_NATIVE_UINT64
hdf5_type_id(::Type{Float16}) = API.H5T_NATIVE_FLOAT16

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hdf5_type_id(::Type{Float16}) = API.H5T_NATIVE_FLOAT16 will fail (at compile time or runtime) if the API constant can’t be loaded on some HDF5 versions/builds. Consider defining this method conditionally (based on API._has_symbol(:H5T_NATIVE_FLOAT16_g) / libversion) and raising a targeted error message when Float16 datatypes are not supported by the linked HDF5 library, or mapping to an available standard half-float type when present.

Suggested change
hdf5_type_id(::Type{Float16}) = API.H5T_NATIVE_FLOAT16
function hdf5_type_id(::Type{Float16})
if API._has_symbol(:H5T_NATIVE_FLOAT16_g)
return API.H5T_NATIVE_FLOAT16
end
throw(ArgumentError("Float16 HDF5 datatypes are not supported by the linked HDF5 library"))
end

Copilot uses AI. Check for mistakes.
hdf5_type_id(::Type{Float32}) = API.H5T_NATIVE_FLOAT
hdf5_type_id(::Type{Float64}) = API.H5T_NATIVE_DOUBLE
hdf5_type_id(::Type{Reference}) = API.H5T_STD_REF_OBJ
Expand Down Expand Up @@ -177,7 +178,9 @@ function _hdf5_type_map(class_id, is_signed, native_size)
end
end
else
return if native_size == 4
return if native_size == 2
Float16
elseif native_size == 4
Float32
elseif native_size == 8
Float64
Expand Down
2 changes: 1 addition & 1 deletion src/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Base.cconvert(
) where {T<:Union{Reference,API.hobj_ref_t,Cvoid}} = Ref(ref)

const BitsType = Union{
Bool,Int8,UInt8,Int16,UInt16,Int32,UInt32,Int64,UInt64,Float32,Float64
Bool,Int8,UInt8,Int16,UInt16,Int32,UInt32,Int64,UInt64,Float16,Float32,Float64
}
const ScalarType = Union{BitsType,Reference}

Expand Down
11 changes: 11 additions & 0 deletions test/plain.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ end
A = randn(3, 5)
write(f, "Afloat64", convert(Matrix{Float64}, A))
write(f, "Afloat32", convert(Matrix{Float32}, A))
write(f, "Afloat16", convert(Matrix{Float16}, A))
Ai = rand(1:20, 2, 4)
write(f, "Aint8", convert(Matrix{Int8}, Ai))
f["Aint16"] = convert(Matrix{Int16}, Ai)
Expand Down Expand Up @@ -176,6 +177,9 @@ end
@test zerodim == 42 && isa(zerodim, Int32)
bloscempty = read(fr, "bloscempty")
@test bloscempty == Int64[] && isa(bloscempty, Vector{Int64})
Af16 = read(fr, "Afloat16")
@test convert(Matrix{Float16}, A) == Af16
@test eltype(Af16) == Float16

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Float16 read/write assertions are unconditional. If the linked HDF5 build doesn’t support half-float (e.g., older libhdf5 versions still allowed by this package’s compat), the tests may fail or even error during package load due to missing type IDs. Please guard this block behind a capability check (e.g., API._has_symbol(:H5T_NATIVE_FLOAT16_g) / a libversion check), or bump the minimum supported HDF5_jll/libhdf5 version accordingly.

Suggested change
Af16 = read(fr, "Afloat16")
@test convert(Matrix{Float16}, A) == Af16
@test eltype(Af16) == Float16
if HDF5.API._has_symbol(:H5T_NATIVE_FLOAT16_g)
Af16 = read(fr, "Afloat16")
@test convert(Matrix{Float16}, A) == Af16
@test eltype(Af16) == Float16
end

Copilot uses AI. Check for mistakes.
Af32 = read(fr, "Afloat32")
@test convert(Matrix{Float32}, A) == Af32
@test eltype(Af32) == Float32
Expand Down Expand Up @@ -611,6 +615,7 @@ end # testset plain
Acmplx = rand(ComplexF64, 3, 5)
write(f, "Acmplx64", convert(Matrix{ComplexF64}, Acmplx))
write(f, "Acmplx32", convert(Matrix{ComplexF32}, Acmplx))
write(f, "Acmplx16", convert(Matrix{ComplexF16}, Acmplx))

dset = create_dataset(
f, "Acmplx64_hyperslab", datatype(Complex{Float64}), dataspace(Acmplx)
Expand All @@ -623,6 +628,7 @@ end # testset plain
@test_throws ErrorException f["_ComplexF64"] = 1.0 + 2.0im
@test_throws ErrorException write(f, "_Acmplx64", convert(Matrix{ComplexF64}, Acmplx))
@test_throws ErrorException write(f, "_Acmplx32", convert(Matrix{ComplexF32}, Acmplx))
@test_throws ErrorException write(f, "_Acmplx16", convert(Matrix{ComplexF16}, Acmplx))
HDF5.enable_complex_support()

close(f)
Expand All @@ -633,6 +639,9 @@ end # testset plain
z_attrs = attributes(fr["ComplexF64"])
@test read(z_attrs["ComplexInt64"]) == 1im

Acmplx16 = read(fr, "Acmplx16")
@test convert(Matrix{ComplexF16}, Acmplx) == Acmplx16
@test eltype(Acmplx16) == ComplexF16

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ComplexF16 datasets are now written/read unconditionally. This will break on HDF5 builds that don’t support Float16 type IDs (since complex compound type creation relies on hdf5_type_id(Float16)). Please make these tests conditional on Float16 datatype support (or update the minimum supported HDF5 version) to avoid failures on older libhdf5 installations.

Copilot uses AI. Check for mistakes.
Acmplx32 = read(fr, "Acmplx32")
@test convert(Matrix{ComplexF32}, Acmplx) == Acmplx32
@test eltype(Acmplx32) == ComplexF32
Expand All @@ -651,6 +660,8 @@ end # testset plain
z = read(fr, "ComplexF64")
@test isa(z, NamedTuple{(:r, :i),Tuple{Float64,Float64}})

Acmplx16 = read(fr, "Acmplx16")
@test eltype(Acmplx16) == NamedTuple{(:r, :i),Tuple{Float16,Float16}}
Acmplx32 = read(fr, "Acmplx32")
@test eltype(Acmplx32) == NamedTuple{(:r, :i),Tuple{Float32,Float32}}
Acmplx64 = read(fr, "Acmplx64")
Expand Down
Loading