NeurodataWithoutBorders/pynwb

[Bug] `NWBHDF5IO.can_read()` fails for missing version

h-mayorquin opened this issue · 3 comments

As in the title, the problem is the following, NWBHDF5IO.can_read() relies on get_nwbfile_version and critically it indexes the output:

return get_nwbfile_version(file)[1][0] >= 2 # Major version of NWB >= 2

However, get_nwbfile_version can also return None and then the indexing fails:

def get_nwbfile_version(**kwargs):

It seems to me that the solution should be that if get_nwbfile_version returns None then NWBHDF5IO.can_read() should return None instead of trying to index.

From the discussion in neuroconv:

catalystneuro/neuroconv#910

then NWBHDF5IO.can_read() should return None instead of trying to index.

I'd say a function called can_read ought to just return -> bool, and so False in the case where no version was able to be found by conventional means

Yeah, I think that makes sense. The only confusing semantics is that get_nwbfile_version returns None when the version can't be found. So maybe the file could be read but there is no version....

But I guess better be strict about it?

From what I understand, if the version can't be found the file is corrupt, not a valid NWBFile, or nothing has been written.

I think returning can_read as False makes sense - I can make a PR to update it so that it does not error if get_nwbfile_version returns None but returns False instead.