[Bug]: nwbinspector flags BEST_PRACTICE_VIOLATION when using ndx-photometry extension

pauladkisson commented 1 year ago

What happened?

Current Behavior

When running nwbinspector on files containing ndx-photometry data, it flags a BEST_PRACTICE_VIOLATION for each ROIResponseSeries with message:

check_roi_response_series_link_to_plane_segmentation - 'RoiResponseSeries' object at location '/processing/ophys/UVReferenceFSmoothed'
       Message: rois field does not point to a PlaneSegmentation table.

Expected Behavior

nwbinspector should not flag any violations since ndx-photometry data does not use PlaneSegmentation tables.

Minimal code to reproduce

See ndx-photometry tutorial to generate the nwbfile and then run in CLI

nwbinspector <nwbfile_path>

Operating System

macOS

Python Version

3.11

Were you streaming with ROS3?

No

Package Versions

attrs 23.1.0 click 8.1.3 h5py 3.8.0 hdmf 3.6.1 isodate 0.6.1 jsonschema 4.17.3 natsort 8.3.1 ndx-photometry 0.2.0 numpy 1.24.3 nwbinspector 0.4.28 packaging 23.1 pandas 2.0.2 pip 23.1.2 pynwb 2.3.2 pyrsistent 0.19.3 python-dateutil 2.8.2 pytz 2023.3 PyYAML 6.0 ruamel.yaml 0.17.31 ruamel.yaml.clib 0.2.7 scipy 1.10.1 setuptools 67.8.0 six 1.16.0 tqdm 4.65.0 tzdata 2023.3 wheel 0.38.4

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

CodyCBakerPhD commented 1 year ago

Yeah, this is a tricky one because the environment in which the NWB Inspector is run is not necessarily one in which the ndx-photometry extension would be installed, so it's hard to add a conditional skip statement to the check function of the form if isinstance(dynamic_table_target, FibersTable)

I guess we could maybe do something less sophisticated but less general, such as if type(dynamic_table_target).__name__ == "FibersTable"?

CodyCBakerPhD commented 1 year ago

I also don't know if it would be a good practice to add ndx-photometry to the minimal requirements of the Inspector...

Any thoughts @rly @oruebel @bendichter? This is the first time we've seen such a clear interaction between the checks on core neurodata types and some extended ones

weiglszonja commented 1 year ago

https://github.com/NeurodataWithoutBorders/pynwb/issues/1777 could be the second one with ndx-miniscope, but that turns out to be a validation error

NeurodataWithoutBorders / nwbinspector