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.

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

Error when file is not found #859

Closed
bkamins opened this issue Jul 28, 2021 · 5 comments · Fixed by #863
Closed

Error when file is not found #859

bkamins opened this issue Jul 28, 2021 · 5 comments · Fixed by #863

Comments

@bkamins
Copy link
Member

bkamins commented Jul 28, 2021

The message in

throw(ArgumentError("\"$source\" is not a valid file"))

is reported to be misleading by the users.

Maybe we could split the isfile condition to a separate test and if file is not found report that it was not found? (users think that "valid file" means that file exists but did not pass validation)

@nickrobinson251
Copy link
Collaborator

nickrobinson251 commented Jul 28, 2021

Do we need to split out the isfile condition or just change the wording in this one check (e.g. to "No such file \"$source\"")?

i.e. does this error ever get triggered except for non-existent filepaths? I tried to trigger it by passing in various silly arguments to CSV.File (e.g. md files, xml files, Vector{UInt16}) and couldn't trigger it -- either i hit a different error (usually isfile throwing an error) or reading succeeded (turn's out CSV.jl will read markdown somewhat successfully!)

@bkamins
Copy link
Member Author

bkamins commented Jul 28, 2021

Indeed isfile will throw an error if something that is not a file is passed to it. However, I think we could consider adding a restriction that we accept AbstractString here (and then throw "file not found") or on any other type throw an error?

The issue is that isfile calls stat which has the following methods:

julia> methods(stat)
# 7 methods for generic function "stat":
[1] stat(s::IOStream) in Base at iostream.jl:57
[2] stat(fd::Base.Libc.WindowsRawSocket) in Base.Filesystem at stat.jl:79
[3] stat(path::AbstractString) in Base.Filesystem at stat.jl:80
[4] stat(fd::RawFD) in Base.Filesystem at stat.jl:83
[5] stat(fd::Integer) in Base.Filesystem at stat.jl:85
[6] stat(f::Base.Filesystem.File) in Base.Filesystem at filesystem.jl:246
[7] stat(path...) in Base.Filesystem at stat.jl:109

And I am not sure all of them are properly handled later by CSV.File, e.g. is this expected:

julia> CSV.File(1)
0-element CSV.File{false}

?

@nickrobinson251
Copy link
Collaborator

we could consider adding a restriction that we accept AbstractString here

I think this would break FilePathsBase.AbstractPath, which are currently supported due to the lack of type constraints (or require us adding a FilePathsBase dependency)

@bkamins
Copy link
Member Author

bkamins commented Jul 28, 2021

I am flexible here - maybe even CSV.File supports all that stat supports - @quinnj will know hopefully.

@quinnj
Copy link
Member

quinnj commented Aug 3, 2021

How about #863?

quinnj added a commit that referenced this issue Aug 3, 2021
* Improve error message in case input file doesn't exist

Attempt to fix #859.

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants