Skip to content

Commit

Permalink
Loosen filename type constraints (#259)
Browse files Browse the repository at this point in the history 
* 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.

* Loosen constraints further and test directly against FilePathsBase.jl

* Code review changes.
  • Loading branch information
@omus
omus authored May 1, 2020
2 parents ffe4340 + 26c9484 commit 882dc75
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 101 deletions.
5 changes: 3 additions & 2 deletions Project.toml
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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"]
15 changes: 6 additions & 9 deletions src/loadsave.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,17 @@ 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(file, args...; options...) = $fn(query(file), 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(file; options...) = data -> save(file, 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}}, 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
Expand All @@ -143,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::AbstractString, 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
Expand Down
6 changes: 2 additions & 4 deletions src/query.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/types.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
16 changes: 9 additions & 7 deletions test/loadsave.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,32 @@ 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)
add_loader(format"HDF5", :TestLoadSave2)
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
Expand Down
139 changes: 72 additions & 67 deletions test/query.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,82 +266,87 @@ finally
merge!(FileIO.sym2info, sym2info)
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_dir = joinpath(@__DIR__, "files")
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")
Expand Down
4 changes: 4 additions & 0 deletions test/runtests.jl
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
using FileIO
using FilePathsBase
using Test

# Both FileIO and FilePathsBase export filename, but we only want the FileIO definition.
using FileIO: filename

struct MimeSaveTestType
end

Expand Down
21 changes: 11 additions & 10 deletions test/test_mimesave.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 comments on commit 882dc75

@omus
Copy link
Member Author

@omus omus commented on 882dc75 May 1, 2020

Choose a reason for hiding this comment

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

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/13982

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v1.3.0 -m "<description of version>" 882dc754b9dce3b32def85a42717eefefb0fd1b6
git push origin v1.3.0

Please sign in to comment.