From 3e4f4e2dacfa0ad7f4d5159bc3de8142e91e7de5 Mon Sep 17 00:00:00 2001 From: rofinn Date: Wed, 29 Apr 2020 10:51:51 -0500 Subject: [PATCH 1/4] Loosen File and Stream filename type constraints. Maintains the same public API (e.g., type constraints on methods remains the same), but allows FilePaths.jl to extend the public methods to work with `AbstractPath`s. This seemed preferable to depending directly on FilePathsBase or loosening the type constraints on all methods. --- src/types.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types.jl b/src/types.jl index 92ffabca..af5d5a61 100644 --- a/src/types.jl +++ b/src/types.jl @@ -26,7 +26,7 @@ DataFormat `fmt`. For example, `File{fmtpng}(filename)` would indicate a PNG file. """ struct File{F<:DataFormat} <: Formatted{F} - filename::String + filename end File(fmt::Type{DataFormat{sym}}, filename) where {sym} = File{fmt}(filename) @@ -53,7 +53,7 @@ be used to improve error messages, etc. """ struct Stream{F <: DataFormat, IOtype <: IO} <: Formatted{F} io::IOtype - filename::Union{String, Nothing} + filename end Stream(::Type{F}, io::IO) where {F<:DataFormat} = Stream{F,typeof(io)}(io, nothing) From 131106257eb47c4bc31ad94a580eed276bc74c78 Mon Sep 17 00:00:00 2001 From: rofinn Date: Wed, 29 Apr 2020 11:54:23 -0500 Subject: [PATCH 2/4] Loosen constraints further and test directly against FilePathsBase.jl --- Project.toml | 3 +- src/loadsave.jl | 11 ++-- src/query.jl | 6 +- test/loadsave.jl | 16 ++--- test/query.jl | 137 ++++++++++++++++++++++-------------------- test/runtests.jl | 5 ++ test/test_mimesave.jl | 21 ++++--- 7 files changed, 104 insertions(+), 95 deletions(-) diff --git a/Project.toml b/Project.toml index 3a1cf25e..a149f96d 100644 --- a/Project.toml +++ b/Project.toml @@ -9,8 +9,9 @@ Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f" julia = "0.7, 1" [extras] +FilePathsBase = "48062228-2e41-5def-b9a4-89aafe57970f" Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [targets] -test = ["Test", "Random"] +test = ["FilePathsBase", "Test", "Random"] diff --git a/src/loadsave.jl b/src/loadsave.jl index 177a89da..5a22aae7 100755 --- a/src/loadsave.jl +++ b/src/loadsave.jl @@ -115,17 +115,14 @@ savestreaming # if a bare filename or IO stream are given, query for the format and dispatch # to the formatted handlers below for fn in (:load, :loadstreaming, :save, :savestreaming, :metadata) - @eval $fn(s::Union{AbstractString,IO}, args...; options...) = - $fn(query(s), args...; options...) + @eval $fn(s, args...; options...) = $fn(query(s), args...; options...) end # return a save function, so you can do `thing_to_save |> save("filename.ext")` -function save(s::Union{AbstractString,IO}; options...) - data -> save(s, data; options...) -end +save(s; options...) = data -> save(s, data; options...) # Allow format to be overridden with first argument -function save(df::Type{DataFormat{sym}}, f::AbstractString, data...; options...) where sym +function save(df::Type{DataFormat{sym}}, f, data...; options...) where sym libraries = applicable_savers(df) checked_import(libraries[1]) return Base.invokelatest(save, File(DataFormat{sym}, f), data...; options...) @@ -143,7 +140,7 @@ function save(df::Type{DataFormat{sym}}, s::IO, data...; options...) where sym return Base.invokelatest(save, Stream(DataFormat{sym}, s), data...; options...) end -function savestreaming(df::Type{DataFormat{sym}}, f::AbstractString, data...; options...) where sym +function savestreaming(df::Type{DataFormat{sym}}, f, data...; options...) where sym libraries = applicable_savers(df) checked_import(libraries[1]) return Base.invokelatest(savestreaming, File(DataFormat{sym}, f), data...; options...) diff --git a/src/query.jl b/src/query.jl index bcb5caf7..cd4cb442 100644 --- a/src/query.jl +++ b/src/query.jl @@ -65,7 +65,7 @@ end `query(filename)` returns a `File` object with information about the format inferred from the file's extension and/or magic bytes. """ -function query(filename::AbstractString) +function query(filename) _, ext = splitext(filename) if haskey(ext2sym, ext) sym = ext2sym[ext] @@ -102,9 +102,7 @@ hasfunction(s::Tuple) = false #has magic `query(io, [filename])` returns a `Stream` object with information about the format inferred from the magic bytes. """ -query(io::IO, filename) = query(io, String(filename)) - -function query(io::IO, filename::Union{Nothing, String} = nothing) +function query(io::IO, filename = nothing) magic = Vector{UInt8}() pos = position(io) for p in magic_list diff --git a/test/loadsave.jl b/test/loadsave.jl index a8e3cba6..cc709876 100644 --- a/test/loadsave.jl +++ b/test/loadsave.jl @@ -22,7 +22,9 @@ try empty!(FileIO.sym2loader) empty!(FileIO.sym2saver) file_dir = joinpath(dirname(@__FILE__), "files") - @testset "Load" begin + file_path = Path(file_dir) + + @testset "Load $(typeof(fp))" for fp in (file_dir, file_path) add_loader(format"PBMText", :TestLoadSave) add_loader(format"PBMBinary", :TestLoadSave) @@ -30,22 +32,22 @@ try add_loader(format"JLD", :TestLoadSave) add_loader(format"GZIP", :TestLoadSave) - @test load(joinpath(file_dir,"file1.pbm")) == "PBMText" - @test load(joinpath(file_dir,"file2.pbm")) == "PBMBinary" + @test load(joinpath(fp,"file1.pbm")) == "PBMText" + @test load(joinpath(fp,"file2.pbm")) == "PBMBinary" # Regular HDF5 file with magic bytes starting at position 0 - @test load(joinpath(file_dir,"file1.h5")) == "HDF5" + @test load(joinpath(fp,"file1.h5")) == "HDF5" # This one is actually a JLD file saved with an .h5 extension, # and the JLD magic bytes edited to prevent it from being recognized # as JLD. # JLD files are also HDF5 files, so this should be recognized as # HDF5. However, what makes this more interesting is that the # magic bytes start at position 512. - @test load(joinpath(file_dir,"file2.h5")) == "HDF5" + @test load(joinpath(fp,"file2.h5")) == "HDF5" # JLD file saved with .jld extension - @test load(joinpath(file_dir,"file.jld")) == "JLD" + @test load(joinpath(fp,"file.jld")) == "JLD" # GZIP file saved with .gz extension - @test load(joinpath(file_dir,"file.csv.gz")) == "GZIP" + @test load(joinpath(fp,"file.csv.gz")) == "GZIP" @test_throws Exception load("missing.fmt") end finally diff --git a/test/query.jl b/test/query.jl index bfb51e03..25128e1a 100644 --- a/test/query.jl +++ b/test/query.jl @@ -267,81 +267,86 @@ finally end file_dir = joinpath(dirname(@__FILE__), "files") -@testset "bedGraph" begin - q = query(joinpath(file_dir, "file.bedgraph")) - @test typeof(q) == File{format"bedGraph"} - open(q) do io - @test position(io) == 0 - skipmagic(io) - @test position(io) == 0 # no skipping for functions - # @test FileIO.detect_bedgraph(io) # MethodError: no method matching readline(::FileIO.Stream{FileIO.DataFormat{:bedGraph},IOStream}; chomp=false) - end - open(joinpath(file_dir, "file.bedgraph")) do io - @test (FileIO.detect_bedgraph(io)) +file_path = Path(file_dir) + +@testset "Querying with $(typeof(fp))" for fp in (file_dir, file_path) + @testset "bedGraph" begin + q = query(joinpath(file_dir, "file.bedgraph")) + @test typeof(q) == File{format"bedGraph"} + open(q) do io + @test position(io) == 0 + skipmagic(io) + @test position(io) == 0 # no skipping for functions + # @test FileIO.detect_bedgraph(io) # MethodError: no method matching readline(::FileIO.Stream{FileIO.DataFormat{:bedGraph},IOStream}; chomp=false) + end + open(joinpath(file_dir, "file.bedgraph")) do io + @test (FileIO.detect_bedgraph(io)) + end end -end -@testset "STL detection" begin - q = query(joinpath(file_dir, "ascii.stl")) - @test typeof(q) == File{format"STL_ASCII"} - q = query(joinpath(file_dir, "binary_stl_from_solidworks.STL")) - @test typeof(q) == File{format"STL_BINARY"} - open(q) do io - @test position(io) == 0 - skipmagic(io) - @test position(io) == 0 # no skipping for functions + @testset "STL detection" begin + q = query(joinpath(file_dir, "ascii.stl")) + @test typeof(q) == File{format"STL_ASCII"} + q = query(joinpath(file_dir, "binary_stl_from_solidworks.STL")) + @test typeof(q) == File{format"STL_BINARY"} + open(q) do io + @test position(io) == 0 + skipmagic(io) + @test position(io) == 0 # no skipping for functions + end end -end -@testset "PLY detection" begin - q = query(joinpath(file_dir, "ascii.ply")) - @test typeof(q) == File{format"PLY_ASCII"} - q = query(joinpath(file_dir, "binary.ply")) - @test typeof(q) == File{format"PLY_BINARY"} + @testset "PLY detection" begin + q = query(joinpath(file_dir, "ascii.ply")) + @test typeof(q) == File{format"PLY_ASCII"} + q = query(joinpath(file_dir, "binary.ply")) + @test typeof(q) == File{format"PLY_BINARY"} -end -@testset "Multiple Magic bytes" begin - q = query(joinpath(file_dir, "magic1.tiff")) - @test typeof(q) == File{format"TIFF"} - q = query(joinpath(file_dir, "magic2.tiff")) - @test typeof(q) == File{format"TIFF"} - open(q) do io - @test position(io) == 0 - skipmagic(io) - @test position(io) == 4 end -end -@testset "AVI Detection" begin - open(joinpath(file_dir, "bees.avi")) do s - @test FileIO.detectavi(s) - end - open(joinpath(file_dir, "sin.wav")) do s - @test !(FileIO.detectavi(s)) + @testset "Multiple Magic bytes" begin + q = query(joinpath(file_dir, "magic1.tiff")) + @test typeof(q) == File{format"TIFF"} + q = query(joinpath(file_dir, "magic2.tiff")) + @test typeof(q) == File{format"TIFF"} + open(q) do io + @test position(io) == 0 + skipmagic(io) + @test position(io) == 4 + end end - open(joinpath(file_dir, "magic1.tiff")) do s - @test !(FileIO.detectavi(s)) + @testset "AVI Detection" begin + open(joinpath(file_dir, "bees.avi")) do s + @test FileIO.detectavi(s) + end + open(joinpath(file_dir, "sin.wav")) do s + @test !(FileIO.detectavi(s)) + end + open(joinpath(file_dir, "magic1.tiff")) do s + @test !(FileIO.detectavi(s)) + end + q = query(joinpath(file_dir, "bees.avi")) + @test typeof(q) == File{format"AVI"} end - q = query(joinpath(file_dir, "bees.avi")) - @test typeof(q) == File{format"AVI"} -end -@testset "RDA detection" begin - q = query(joinpath(file_dir, "minimal_ascii.rda")) - @test typeof(q) == File{format"RData"} - open(q) do io - @test position(io) == 0 - @test FileIO.detect_rdata(io) - # 6 for /r/n and 5 for /n - @test (position(io) in (5, 6)) + @testset "RDA detection" begin + q = query(joinpath(file_dir, "minimal_ascii.rda")) + @test typeof(q) == File{format"RData"} + open(q) do io + @test position(io) == 0 + @test FileIO.detect_rdata(io) + # 6 for /r/n and 5 for /n + @test (position(io) in (5, 6)) + end end -end -@testset "RDS detection" begin - q = query(joinpath(file_dir, "minimal_ascii.rds")) - @test typeof(q) == File{format"RDataSingle"} - open(q) do io - @test position(io) == 0 - @test FileIO.detect_rdata_single(io) - # need to seek to beginning of file where data structure starts - @test position(io) == 0 + @testset "RDS detection" begin + q = query(joinpath(file_dir, "minimal_ascii.rds")) + @test typeof(q) == File{format"RDataSingle"} + open(q) do io + @test position(io) == 0 + @test FileIO.detect_rdata_single(io) + # need to seek to beginning of file where data structure starts + @test position(io) == 0 + end end end + @testset "Format with function for magic bytes" begin add_format(format"FUNCTION_FOR_MAGIC_BYTES", x -> 0x00, ".wav", [:WAV]) del_format(format"FUNCTION_FOR_MAGIC_BYTES") diff --git a/test/runtests.jl b/test/runtests.jl index cb528919..354e8673 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,6 +1,11 @@ using FileIO +using FilePathsBase using Test +# Because both FileIO and FilePathsBase export filename, but for our tests we only want the +# FileIO definition. +filename(x) = FileIO.filename(x) + struct MimeSaveTestType end diff --git a/test/test_mimesave.jl b/test/test_mimesave.jl index 08a5b1ee..ac44f456 100644 --- a/test/test_mimesave.jl +++ b/test/test_mimesave.jl @@ -30,21 +30,22 @@ end data = MimeSaveTestType() -output_filename = tempname() +# Test with string and paths +for output_filename in (tempname(), tmpname()) + for filetype in [".svg", ".pdf", ".eps", ".png", ".html"] -for filetype in [".svg", ".pdf", ".eps", ".png", ".html"] + try + save(output_filename * filetype, data) - try - save(output_filename * filetype, data) + content_original = read(joinpath(@__DIR__, "files", "mimesavetest$filetype")) + content_new = read(output_filename * filetype) - content_original = read(joinpath(@__DIR__, "files", "mimesavetest$filetype")) - content_new = read(output_filename * filetype) + @test content_new == content_original + finally + isfile(output_filename * filetype) && rm(output_filename * filetype) + end - @test content_new == content_original - finally - isfile(output_filename * filetype) && rm(output_filename * filetype) end - end end From 926b5a4fa5c438deb3ca21691891049831d31d97 Mon Sep 17 00:00:00 2001 From: rofinn Date: Wed, 29 Apr 2020 15:48:33 -0500 Subject: [PATCH 3/4] Code review changes. --- src/loadsave.jl | 8 ++++---- test/query.jl | 2 +- test/runtests.jl | 5 ++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/loadsave.jl b/src/loadsave.jl index 5a22aae7..c3b14af8 100755 --- a/src/loadsave.jl +++ b/src/loadsave.jl @@ -122,10 +122,10 @@ end save(s; options...) = data -> save(s, data; options...) # Allow format to be overridden with first argument -function save(df::Type{DataFormat{sym}}, f, data...; options...) where sym +function save(df::Type{DataFormat{sym}}, filename, data...; options...) where sym libraries = applicable_savers(df) checked_import(libraries[1]) - return Base.invokelatest(save, File(DataFormat{sym}, f), data...; options...) + return Base.invokelatest(save, File(DataFormat{sym}, filename), data...; options...) end function savestreaming(df::Type{DataFormat{sym}}, s::IO, data...; options...) where sym @@ -140,10 +140,10 @@ function save(df::Type{DataFormat{sym}}, s::IO, data...; options...) where sym return Base.invokelatest(save, Stream(DataFormat{sym}, s), data...; options...) end -function savestreaming(df::Type{DataFormat{sym}}, f, data...; options...) where sym +function savestreaming(df::Type{DataFormat{sym}}, filename, data...; options...) where sym libraries = applicable_savers(df) checked_import(libraries[1]) - return Base.invokelatest(savestreaming, File(DataFormat{sym}, f), data...; options...) + return Base.invokelatest(savestreaming, File(DataFormat{sym}, filename), data...; options...) end # do-syntax for streaming IO diff --git a/test/query.jl b/test/query.jl index 25128e1a..3ffcdc56 100644 --- a/test/query.jl +++ b/test/query.jl @@ -266,7 +266,7 @@ finally merge!(FileIO.sym2info, sym2info) end -file_dir = joinpath(dirname(@__FILE__), "files") +file_dir = joinpath(@__DIR__, "files") file_path = Path(file_dir) @testset "Querying with $(typeof(fp))" for fp in (file_dir, file_path) diff --git a/test/runtests.jl b/test/runtests.jl index 354e8673..aa39fb21 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2,9 +2,8 @@ using FileIO using FilePathsBase using Test -# Because both FileIO and FilePathsBase export filename, but for our tests we only want the -# FileIO definition. -filename(x) = FileIO.filename(x) +# Both FileIO and FilePathsBase export filename, but we only want the FileIO definition. +using FileIO: filename struct MimeSaveTestType end From 26c94846601cc91b9b60fdad5ff00fa05efa5840 Mon Sep 17 00:00:00 2001 From: rofinn Date: Fri, 1 May 2020 11:27:18 -0500 Subject: [PATCH 4/4] Bump minor version number. --- Project.toml | 2 +- src/loadsave.jl | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Project.toml b/Project.toml index a149f96d..f68d6550 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "FileIO" uuid = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549" -version = "1.2.4" +version = "1.3.0" [deps] Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f" diff --git a/src/loadsave.jl b/src/loadsave.jl index c3b14af8..3b308b12 100755 --- a/src/loadsave.jl +++ b/src/loadsave.jl @@ -115,11 +115,11 @@ savestreaming # if a bare filename or IO stream are given, query for the format and dispatch # to the formatted handlers below for fn in (:load, :loadstreaming, :save, :savestreaming, :metadata) - @eval $fn(s, args...; options...) = $fn(query(s), args...; options...) + @eval $fn(file, args...; options...) = $fn(query(file), args...; options...) end # return a save function, so you can do `thing_to_save |> save("filename.ext")` -save(s; options...) = data -> save(s, data; options...) +save(file; options...) = data -> save(file, data; options...) # Allow format to be overridden with first argument function save(df::Type{DataFormat{sym}}, filename, data...; options...) where sym