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.

Sign up for GitHub

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

Add some methods to make JLSOFile more Dict-like #108

Merged
merged 5 commits into from
Jun 2, 2021
Merged

Add some methods to make JLSOFile more Dict-like #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47eacd3
Define `keys/haskey/get/get!` for JLSOFile
nickrobinson251 Jun 2, 2021
2da7917
Fix no-arg constructor for JLSOFile
nickrobinson251 Jun 2, 2021
0099042
Allow `save` to take a JLSOFile
nickrobinson251 Jun 2, 2021
0a08047
fixup! Allow `save` to take a JLSOFile
nickrobinson251 Jun 2, 2021
7c75b90
Bump version
nickrobinson251 Jun 2, 2021
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
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name = "JLSO"
uuid = "9da8a3cd-07a3-59c0-a743-3fdc52c30d11"
license = "MIT"
authors = ["Invenia Technical Computing Corperation"]
version = "2.5.0"
version = "2.6.0"

[deps]
BSON = "fbb218c0-5317-5bc6-957e-2ee96dd4b1f0"
Expand Down
18 changes: 17 additions & 1 deletion src/JLSOFile.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ function JLSOFile(;
image=_image(),
kwargs...
)
data = isempty(kwargs) ? Dict{Symbol,Any}() : Dict(kwargs)
return JLSOFile(
Dict(kwargs);
data;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't love this magic keyword constructor, which has some "special" keywords (version, julia, ...) and then eats all the others as key/value pairs to be serialised... especially as we have a :key => value pair constructor already: JLSOFile(data::Pair{Symbol}...; kwargs...) = JLSOFile(Dict(data); kwargs...)

But I think removing this is another issue (and breaking). Maybe we should open an issue about that.

But whether we have magic-keywords or not, i think this is now the correct behjaviour for JLSOFile() (before it was a cryptic error)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, :+1 for just using pairs for data objects. I think this was proposed cause some folks like creating dataframes with the keyword constructor and we often don't change the special keywords. Please open an issue to delete this method as it should be easy enough to deprecate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#109

version=version,
julia=julia,
format=format,
Expand Down Expand Up @@ -117,3 +118,18 @@ function Base.:(==)(a::JLSOFile, b::JLSOFile)
end

Base.names(jlso::JLSOFile) = collect(keys(jlso.objects))

Base.keys(jlso::JLSOFile) = keys(jlso.objects)

Base.haskey(jlso::JLSOFile, key) = haskey(jlso.objects, key)

Base.get(jlso::JLSOFile, key, default) = haskey(jlso, key) ? jlso[key] : default

Base.get!(jlso::JLSOFile, key, default) = get!(() -> default, jlso, key)
function Base.get!(func, jlso::JLSOFile, key)
return if haskey(jlso, key)
jlso[key]
else
jlso[key] = func()
end
end
3 changes: 2 additions & 1 deletion src/file_io.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ end

Creates a JLSOFile with the specified data and kwargs and writes it back to the io.
"""
save(io::IO, data; kwargs...) = write(io, JLSOFile(data; kwargs...))
save(io::IO, data::JLSOFile) = write(io, data)
save(io::IO, data; kwargs...) = save(io, JLSOFile(data; kwargs...))
nickrobinson251 marked this conversation as resolved.
Show resolved Hide resolved
save(io::IO, data::Pair...; kwargs...) = save(io, Dict(data...); kwargs...)
function save(path::Union{AbstractPath, AbstractString}, args...; kwargs...)
return open(io -> save(io, args...; kwargs...), path, "w")
Expand Down
31 changes: 31 additions & 0 deletions test/JLSOFile.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
@test jlso[:b] == "hello"
@test haskey(jlso.manifest, "BSON")
end

@testset "no-arg constructor" begin
jlso = JLSOFile()
@test jlso isa JLSOFile
@test isempty(jlso.objects)
@test haskey(jlso.manifest, "BSON")
end
end

@testset "unknown format" begin
Expand Down Expand Up @@ -55,3 +62,27 @@ end
@test Pkg.TOML.parsefile(joinpath(d, "Manifest.toml")) == jlso.manifest
end
end

@testset "keys/haskey" begin
jlso = JLSOFile(:string => datas[:String])
@test collect(keys(jlso)) == [:string]
@test haskey(jlso, :string)
@test !haskey(jlso, :other)
end

@testset "get/get!" begin
nickrobinson251 marked this conversation as resolved.
Show resolved Hide resolved
v = datas[:String]
jlso = JLSOFile(:str => v)
@test get(jlso, :str, "fail") == v
@test get!(jlso, :str, "fail") == v

@test get(jlso, :other, v) == v
@test !haskey(jlso, :other)

@test get!(jlso, :other, v) == v
@test jlso[:other] == v

# key must be a Symbol
@test get(jlso, "str", 999) == 999
@test_throws MethodError get!(jlso, "str", 999)
end