Fixes issue 26 and 84 #90
Conversation
Hopefully correctly this time around, first take was PR JuliaIO#29.
| @@ -105,6 +105,6 @@ load(x) = raise_recursive(parse(x)) | |||
|
|
|||
| function roundtrip(x) | |||
| buf = IOBuffer() | |||
| bson(buf, Dict(:data => x)) | |||
| bson(buf, BSONDict(:data => x)) | |||
There was a problem hiding this comment.
Once the lower was fixed, this caused another test failure.
There was a problem hiding this comment.
Shouldn't
bson(buf, Dict(:data => x))not working be considered a bug?
There was a problem hiding this comment.
The failure it caused was:
Line 54 in 7f15a4b
(it errored).
But note that bson(buf, Dict(:data=>x)) does work.
There was a problem hiding this comment.
But note that bson(buf, Dict(:data=>x)) does work.
but it wouldn't work for x = T(()). I'm not sure about the implications but it doesn't seem safe to break this right?
There was a problem hiding this comment.
Yep, you're right. Here the example:
julia> using BSON
julia> struct T{A}
x::A
end
julia> x = T(())
T{Tuple{}}(())
julia> buf = IOBuffer()
IOBuffer(data=UInt8[...], readable=true, writable=true, seekable=true, append=false, size=0, maxsize=Inf, ptr=1, mark=-1)
julia> bson(buf, Dict(:data => x))
julia> BSON.load(seek(buf, 0))[:data]
ERROR: MethodError: no method matching T{Tuple{}}()
Closest candidates are:
T{Tuple{}}(::Any) where A at REPL[2]:2Well, you seem to have a better grasp of this than me, maybe you can take this (or something else) and fix it?
There was a problem hiding this comment.
I don't have any solutions, I'm not familiar with the internals. Let's leave this PR open hoping someone comes in with the correct solution at some point
There was a problem hiding this comment.
Then the question is whether this "bug" is bigger than the bug #26. Otherwise merging this might still make sense.
| @@ -1,6 +1,3 @@ | |||
| lower(x::Dict{Symbol}) = BSONDict(x) | |||
| lower(x::BSONDict) = BSONDict(x) | |||
|
|
|||
There was a problem hiding this comment.
This caused the bug seen in #84.
There was a problem hiding this comment.
Yeah, its a bit of a hack, and not a very good one either.
|
Note sure what the test-failure is about. One thing to consider is (and maybe test) is what happens when one wants to store a As a bit of a side-note: I don't know the code of BSON at all (apart from this PR), nor have a really spent time contemplating it's design. So, I will not be very useful for anything more than band-aids. |
Hopefully correctly this time around, first take was PR #29.