NeurodataWithoutBorders / nwbinspector

Tool to help inspect NWB files for compliance with NWB Best Practices
https://nwbinspector.readthedocs.io/
Other
17 stars 10 forks source link

[Bug]: No module named 'typing_extensions' #522

Closed rly closed 1 month ago

rly commented 1 month ago

What happened?

On a clean install of the main branch, running nwbinspector --config dandi path/to/file.nwb results in:

Traceback (most recent call last):
  File "/Users/rly/mambaforge/envs/test/bin/nwbinspector", line 5, in <module>
    from nwbinspector._nwbinspector_cli import _nwbinspector_cli
  File "/Users/rly/Documents/NWB/nwbinspector/src/nwbinspector/__init__.py", line 3, in <module>
    from ._registration import available_checks, register_check
  File "/Users/rly/Documents/NWB/nwbinspector/src/nwbinspector/_registration.py", line 11, in <module>
    from typing_extensions import Callable
ModuleNotFoundError: No module named 'typing_extensions'

This reference was just added in https://github.com/NeurodataWithoutBorders/nwbinspector/pull/520

I'm surprised the GitHub Actions tests passed. Perhaps typing_extensions is installed earlier in the GH Actions workflow. In this workflow, it looks like pip install ".[dandi]" is called. This installs typing_extensions, unlike pip install . I have not looked at all the workflows. If they all use pip install ".[dandi]", I think we should have some tests without the [dandi] flag too.

Operating System

Linux

Python Version

3.12

Were you streaming with ROS3?

None

Package Versions

No response

Code of Conduct

stephprince commented 1 month ago

It looks like all the workflows use the pip install ".[dandi]" as you mentioned.

That makes sense to check that running without the dandi flag works as well. We could add a separate job or workflow file similar to testing.yml that runs without the dandi flag. I'm not sure if we need to do that across os and python versions since I think that would be running many of the unit tests twice, but maybe at least the latest python?

rly commented 1 month ago

Fixed by #523