[Bug]: `check_timestamps_match_first_dimension` is triggered for `ImageSeries` when external format and timestamps

weiglszonja commented 1 year ago

What happened?

I created an ImageSeries with external file format and timestamps (data is not specified), after writing such an ImageSeries to NWB and running nwbinspector I get the following message:

0.8 stub_functional_scan_17797_4_10_v2.nwb: check_timestamps_match_first_dimension - 'ImageSeries' object at location '/acquisition/Video: stimulus_17797_4_10_v4' Message: The length of the first dimension of data (0) does not match the length of timestamps (381078).

Steps to Reproduce

from pynwb.testing.mock.file import mock_NWBFile
from pynwb.image import ImageSeries
from pynwb import NWBHDF5IO
from nwbinspector import inspect_nwb

image = ImageSeries(
    name="test_image_series",
    description="no description",
    format="external",
    external_file=["something"],
    timestamps=[0,1,2,3],
)

nwb = mock_NWBFile()
nwb.add_acquisition(image)

with NWBHDF5IO("test_image_series.nwb", "w") as io:
    io.write(nwb)

results = list(inspect_nwb(nwbfile_path="test_image_series.nwb"))

from nwbinspector.inspector_tools import format_messages

print("\n".join(format_messages(results, levels=["importance", "file_path"])))

Traceback

No response

Operating System

Linux

Python Executable

Python

Python Version

3.10

Usage

Library (Python code)

Were you streaming with ROS3?

No

Package Versions

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] Have you ensured this bug was not already reported?
[X] To the best of your ability, have you ensured this is a bug within the code that checks the NWBFile, rather than a bug in the NWBFile reader (e.g., PyNWB or MatNWB)?

bendichter commented 1 year ago

Thanks for reporting this, @weiglszonja. Do you think you can fix it?

weiglszonja commented 1 year ago

thanks, yes, I'll try to open a PR by tomorrow.

CodyCBakerPhD commented 1 year ago

Interestingly, we have tests that this is intended behavior for TimeSeries whose data is explicitly set:

https://github.com/NeurodataWithoutBorders/nwbinspector/blob/dev/tests/unit_tests/test_time_series.py#L176-L193

and I know we must have previously considered the special case of ImageSeries with external mode, but alas no actual tests for that skip...

weiglszonja commented 1 year ago

Yeah I noticed, so the correct fix is to check when external mode is on, and data is empty, skip it.

NeurodataWithoutBorders / nwbinspector