Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
3 tasks done
pauladkisson opened this issue Jun 15, 2023 · 3 comments
Open
3 tasks done
Labels
category: bug errors in the code or code behavior

Comments

@pauladkisson
Copy link
Collaborator

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

  • I agree to follow this project's Code of Conduct
  • Have you ensured this bug was not already reported?
  • 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
Copy link
Contributor

CodyCBakerPhD commented Jun 16, 2023

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
Copy link
Contributor

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
Copy link
Contributor

weiglszonja commented Sep 27, 2023

NeurodataWithoutBorders/pynwb#1777 could be the second one with ndx-miniscope, but that turns out to be a validation error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug errors in the code or code behavior
Projects
None yet
Development

No branches or pull requests

3 participants