Return can_read as False if no nwbfile version is found

stephprince commented 4 months ago

Motivation

Fix #1919. The can_read method would previously throw a TypeError if no nwbfile version was found. It now returnsFalse instead.

I also added additional tests for can_read to improve code coverage.

How to test the behavior?

from h5py import File
from pynwb import NWBHDF5IO
from pynwb.testing.mock.file import mock_NWBFile

nwbfile = mock_NWBFile()
path = 'test.nwb'

with NWBHDF5IO(path, 'w') as io:
    io.write(nwbfile)

with File(path, mode='a') as io:
    del io.attrs['nwb_version']

print(f'{NWBHDF5IO.can_read(path) = }')

Checklist

[x] Did you update CHANGELOG.md with your changes?
[X] Have you checked our Contributing document?
[X] Have you ensured the PR clearly describes the problem and the solution?
[X] Is your contribution compliant with our coding style? This can be checked running ruff check . && codespell from the source directory.
[X] Have you checked to ensure that there aren't other open Pull Requests for the same change?
[X] Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.20%. Comparing base (570fb3b) to head (c69688c). Report is 17 commits behind head on dev.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #1934 +/- ## ========================================== + Coverage 91.91% 92.20% +0.28% ========================================== Files 27 27 Lines 2648 2656 +8 Branches 691 693 +2 ========================================== + Hits 2434 2449 +15 + Misses 141 134 -7 Partials 73 73 ``` | [Flag](https://app.codecov.io/gh/NeurodataWithoutBorders/pynwb/pull/1934/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NeurodataWithoutBorders) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/NeurodataWithoutBorders/pynwb/pull/1934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NeurodataWithoutBorders) | `73.15% <100.00%> (+0.34%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/NeurodataWithoutBorders/pynwb/pull/1934/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NeurodataWithoutBorders) | `83.77% <11.11%> (-0.22%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NeurodataWithoutBorders#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mavaylon1 commented 4 months ago

@stephprince It may be informative to have a distinction between an old file and an invalid file. I think having a warning plus the return false would be better for old files.

stephprince commented 4 months ago

@mavaylon1 added some warnings for when the file can be read by h5py but not PyNWB

h-mayorquin commented 4 months ago

Thanks for working on this guys.

NeurodataWithoutBorders / pynwb