[Bug]: ImageSeries with external_file causes a nwbinspector error

lawrence-mbf commented 1 year ago

What happened?

Error Trace

``` 0 ERROR ======== 0.0 inspector_err.nwb: check_empty_string_for_optional_attribute - 'None' object with name 'None' Message: Traceback (most recent call last): File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\nwbinspector.py", line 650, in run_checks output = robust_s3_read(command=check_function, command_args=[nwbfile_object]) File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\utils.py", line 173, in robust_s3_read raise exc File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\utils.py", line 168, in robust_s3_read return command(*command_args, **command_kwargs) File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\register_checks.py", line 133, in auto_parse_some_output for result in output: File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\checks\nwb_containers.py", line 80, in check_empty_string_for_optional_attribute fields = [attr for attr in optional_attrs if getattr(nwb_container, attr) == ""] File "C:\Users\lawrence\AppData\Local\miniconda3\envs\nwb-inspector\lib\site-packages\nwbinspector\checks\nwb_containers.py", line 80, in fields = [attr for attr in optional_attrs if getattr(nwb_container, attr) == ""] ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all() ```

Operating System

Windows

Python Version

3.10

Were you streaming with ROS3?

No

Package Versions

Python Environment

``` Package Version ------------------------- -------- attrs 23.1.0 click 8.1.6 colorama 0.4.6 h5py 3.9.0 hdmf 3.8.1 isodate 0.6.1 jsonschema 4.18.4 jsonschema-specifications 2023.7.1 natsort 8.4.0 numpy 1.25.1 nwbinspector 0.4.29 packaging 23.1 pandas 2.0.3 pip 23.2.1 pynwb 2.4.0 python-dateutil 2.8.2 pytz 2023.3 PyYAML 6.0.1 referencing 0.30.0 rpds-py 0.9.2 ruamel.yaml 0.17.32 ruamel.yaml.clib 0.2.7 scipy 1.11.1 setuptools 68.0.0 six 1.16.0 tqdm 4.65.0 tzdata 2023.3 wheel 0.38.4 ```

This file was created by the latest MatNWB release on MATLAB R2023a

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)?

lawrence-mbf commented 1 year ago

MatNWB script to recreate the file

```MATLAB nwb = NwbFile(... 'session_start_time', datetime('now', 'TimeZone', 'local'), ... 'identifier', 'MatNWBInspectorError', ... 'session_description', 'inspector error'... ); external_files = {'video1.pmp4', 'video2.pmp4'}; timestamps = [0.0, 0.04, 0.07, 0.1, 0.14, 0.16, 0.21]; behavior_external_file = types.core.ImageSeries( ... 'description', 'Behavior video of animal moving in environment', ... 'data_unit', 'n.a.', ... 'external_file', external_files, ... 'format', 'external', ... 'external_file_starting_frame', [0, 2, 4], ... 'timestamps', timestamps ... ); nwb.acquisition.set('ExternalVideos', behavior_external_file); nwbExport(nwb, 'inspector_err.nwb'); ```

CodyCBakerPhD commented 1 year ago

A couple things here I noticed when making a PyNWB mock example

i) Don't you need a name field specified to make the neurodata object in MatNWB? ii) Shouldn't it be unit instead of data_unit? iii) Shouldn't it be starting_frame instead of external_file_starting_frame? iv) starting_frame should match the length of external_files (PyNWB actually validates this and prevents creation)

Otherwise, here's the equivalent PyNWB code

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

nwbfile = mock_NWBFile()

image_series = ImageSeries(
    name="TestImageSeries",
    description="Behavior video of animal moving in environment",
    unit="n.a.",
    external_file=["test1.mp4", "test2.avi"],
    format="external",
    starting_frame=[0, 2],
    timestamps=[0.0, 0.04, 0.07, 0.1, 0.14, 0.16, 0.21],
)
nwbfile.add_acquisition(image_series)

nwbfile_path = ".../test_nwbinspector_issue.nwb"
with NWBHDF5IO(path=nwbfile_path, mode="w") as io:
    io.write(nwbfile)

results = list(inspect_nwbfile(nwbfile_path=nwbfile_path))

at which point I'm unable to reproduce the issue

Are you able to share your inspector_err.nwb file directly?

Otherwise it will take me a bit more time to setup MatNWB on my device to run the code; but I already suspect that this is simply the result of differences in how MatNWB vs. PyNWB write this data type

lawrence-mbf commented 1 year ago

i) Don't you need a name field specified to make the neurodata object in MatNWB?

Not until you actually add the object to a NWB file so technically the name is defined in this line:

nwb.acquisition.set('ExternalVideos', behavior_external_file);

ii) Shouldn't it be unit instead of data_unit? iii) Shouldn't it be starting_frame instead of external_file_starting_frame?

MatNWB flattens attribute names to their datasets. This design allows us to avoid formalizing full HDF wrappers for generic "Group" and "Dataset" objects.

Are you able to share your inspector_err.nwb file directly?

download link to inspector.err

CodyCBakerPhD commented 1 year ago

Thanks for explaining the MatNWB quirks~

I'm dumb BTW, I forgot to add the image series to the file in the code above; I can now reproduce the problem

NeurodataWithoutBorders / nwbinspector