Skip to content

Commit

Permalink
Fix string vlens
Browse files Browse the repository at this point in the history
  • Loading branch information
@simonster
simonster committed Apr 6, 2015
1 parent 87e343c commit 245639e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/plain.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
module HDF5

using Compat
using Compat: unsafe_convert

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Apr 8, 2015

Contributor

For me, this doesn't work. I need to change using to import, otherwise I get an error upon loading a file. For example:

ERROR: LoadError: LoadError: LoadError: UndefVarError: unsafe_convert not defined
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in reload_path at ./loading.jl:153
 in _require at ./loading.jl:68
 in require at ./loading.jl:54
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in reload_path at ./loading.jl:153
 in reload at ./loading.jl:86
while loading /home/carlo/.julia/v0.4/HDF5/src/plain.jl, in expression starting on line 8
while loading /home/carlo/.julia/v0.4/HDF5/src/HDF5.jl, in expression starting on line 1

Anyone else seeing this?

This comment has been minimized.

Sign in to view

Copy link
@simonster

simonster Apr 8, 2015

Author Member

Are you sure Compat is up to date?

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Apr 8, 2015

Contributor

Yes, and actually I started seeing this right after updating. Plus, if I change using to import it works fine (note: in the previous message I mistakenly wrote include rather than import)

(Actually, I'm not even sure what the syntax using Modulename: function means exactly in general, and how it differs from import.)

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Apr 8, 2015

Contributor

To be more precise, and for posterity: I'm on Compat version 0.3.7.

This comment has been minimized.

Sign in to view

Copy link
@simonster

simonster Apr 8, 2015

Author Member

In theory the difference is the same as the standard using/import distinction: the former doesn't allow extending the bindings, whereas the latter does. But that distinction is not actually important in this case, and using import would be just fine.

I am kind of puzzled that using wouldn't work, though. I tried this on two systems (both 0.3 and 0.4 on OS X and Linux) and it loads on both. Also seems to work on Travis and AppVeyor.

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Apr 8, 2015

Contributor

Ah of course, thanks.

I'll try to investigate a some more and see if I manage to get a small reproducible test case.

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Apr 9, 2015

Contributor

I updated julia to the latest master and now it seems to work. Still not sure what was the issue... oh well

## Add methods to...
import Base: close, convert, done, dump, eltype, endof, flush, getindex,
Expand Down Expand Up @@ -458,7 +459,7 @@ function vlenpack{T<:Union(HDF5BitsKind,CharType)}(v::HDF5Vlen{T})
Tp = t2p(T) # Ptr{UInt8} or Ptr{T}
h = Array(Hvl_t, len)
for i = 1:len
h[i] = Hvl_t(convert(Csize_t, length(v.data[i])), convert(Ptr{Void}, convert(Tp, v.data[i])))
h[i] = Hvl_t(convert(Csize_t, length(v.data[i])), convert(Ptr{Void}, unsafe_convert(Tp, v.data[i])))
end
h
end
Expand Down Expand Up @@ -1365,7 +1366,7 @@ end
atype{T<:HDF5BitsKind}(::Type{T}) = Array{T}
atype{C<:CharType}(::Type{C}) = stringtype(C)
p2a{T<:HDF5BitsKind}(p::Ptr{T}, len::Int) = pointer_to_array(p, len, true)
p2a{C<:CharType}(p::Ptr{C}, len::Int) = stringtype(C)(pointer_to_array(p, len, true))
p2a{C<:CharType}(p::Ptr{C}, len::Int) = stringtype(C)(pointer_to_array(convert(Ptr{UInt8}, p), len, true))
t2p{T<:HDF5BitsKind}(::Type{T}) = Ptr{T}
t2p{C<:CharType}(::Type{C}) = Ptr{UInt8}
function read{T<:Union(HDF5BitsKind,CharType)}(obj::DatasetOrAttribute, ::Type{HDF5Vlen{T}})
Expand Down
5 changes: 5 additions & 0 deletions test/plain.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ end
# Arrays of strings
salut_split = ["Hi", "there"]
write(f, "salut_split", salut_split)
# Arrays of strings as vlen
vlen = HDF5Vlen(salut_split)
d_write(f, "salut_vlen", vlen)
# Empty arrays
empty = Array(UInt32, 0)
write(f, "empty", empty)
Expand Down Expand Up @@ -135,6 +138,8 @@ ucoder = read(fr, "ucode")
@assert ucode == ucoder
salut_splitr = read(fr, "salut_split")
@assert salut_splitr == salut_split
salut_vlenr = read(fr, "salut_vlen")
@assert salut_vlenr == salut_split
Rr = read(fr, "mygroup/CompressedA")
@assert Rr == R
Rr2 = read(fr, "mygroup2/CompressedA")
Expand Down

1 comment on commit 245639e

@simonster
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems Compat.jl needs to be bumped before this will work.

Please sign in to comment.