Tries to parse non-existent file

[stellarpower]

After hitting #66 and trying again using Ruby, I realise the path I actually gave EDF.read was a folder, and not a file! Checks on the path and basic opening of the file's content should be made before trying to parse the structures inside.

[palday]

The lack of check is actually an intentional design decision:

EDF.jl/src/read.jl

Line 297 in 19e8549

read(path) = open(read, path)

This way, we can support anything that open supports, which may or may not be traditional paths on a local filesystem but instead things like paths on S3. S3 is a key-value store so technically there aren't directories and files, although the paths/names are often interpreted as hierarchical.

Calling isdir or isfile would require that appropriate methods are defined for these objects, but we don't actually care what an entity is as long as it's readable with Base.open(read, path).

On checking the content of the EDF ... well, trying to parse the file is in some sense the ultimate check of its content. 😄

[stellarpower]

That all definitely makes sense. But at the same time, trying to read the contents of a local folder definitely is nonsensical and not physically possible, so I think it confuses the situation to get an error message back complaining about a timestamp that couldn't be parsed. Maybe the fault here is with Base.open then. I can't see how you'd ever be able to get a stream of binary or textual data from a folder on any major OS, except some weirdness like reading the extended attributes.

[ararslan]

Maybe the fault here is with Base.open then. I can't see how you'd ever be able to get a stream of binary or textual data from a folder on any major OS

Indeed, you can't:

julia> open(read, "Downloads/")
UInt8[]

But the ability to Base.open a directory is a feature, not a bug. It allows obtaining the file descriptor for the IOStream, which in turn facilitates the use of stat, ispath, mtime, etc.

The issue here is that EDF.jl is calling read(io, n) and assuming it will get exactly n bytes back, but the read(::IOStream, ::Integer) method only guarantees you'll receive at most n bytes. Something we could do is replace the first use of String(read(io, n)) when parsing the file with something like

initial_bytes = read(io, n)
length(initial_bytes) < n && throw(EOFError())
whatever = String(initial_bytes)

An end of file error could at least be less cryptic than complaining about not being able to parse a date.

[stellarpower]

I was thinking along the same lines. Let's say a file doesn't exist, or we open /dev/null, or the cases mentioned above - if the behaviour of Base.open is not necessarily ideal here, I think the lowest common denominator is that an EDF file, whether we read it all in or not, has a header and the length of that header is fixed or at least has a minimum (correct me if I'm wrong). We need a minimal number of bytes to be able to try and parse it; if we can't read in enough (or any) bytes to perform these steps then I think that is the point we can throw an error saying there's been some issue that is down to the filesystem/other storage backend and not due to reading bytes but not getting a valid EDF file.