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]: Error handling in nwb.py masks true error #241

Closed
3 tasks done
rly opened this issue Nov 22, 2024 · 2 comments · Fixed by #242
Closed
3 tasks done

[Bug]: Error handling in nwb.py masks true error #241

rly opened this issue Nov 22, 2024 · 2 comments · Fixed by #242
Labels
category: bug errors in the code or code behavior

Comments

@rly
Copy link
Contributor

rly commented Nov 22, 2024

What happened?

NWBZarrIO is wrapped in a giant try/except that masks exceptions: https://github.com/hdmf-dev/hdmf-zarr/blob/dev/src/hdmf_zarr/nwb.py#L12

pynwb 2.8.2 is not compatible with the dev version of HDMF. When using pynwb < 2.8.3 (just released) with the dev version of HDMF (which will soon be released), and trying to import NWBZarrIO, you get:

ImportError while importing test module '/home/runner/work/hdmf-zarr/hdmf-zarr/tests/unit/test_fsspec_streaming.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.13.0/x64/lib/python3.13/importlib/__init__.py:88: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/test_fsspec_streaming.py:2: in <module>
    from hdmf_zarr import NWBZarrIO
/opt/hostedtoolcache/Python/3.13.0/x64/lib/python3.13/site-packages/hdmf_zarr/__init__.py:3: in <module>
    from .nwb import NWBZarrIO
E   ImportError: cannot import name 'NWBZarrIO' from 'hdmf_zarr.nwb' (/opt/hostedtoolcache/Python/3.13.0/x64/lib/python3.13/site-packages/hdmf_zarr/nwb.py)

The warning doesn't even show up for some reason, and the error hides the true error.

This shows up in the HDMF dev compatibility tests currently, until the pynwb requirement in hdmf-zarr is updated to 2.8.3.

The error should be more informative, and the import should perhaps be refactored.

Steps to Reproduce

from .nwb import NWBZarrIO

Traceback

See above

Operating System

macOS

Python Executable

Conda

Python Version

3.12

Package Versions

No response

Code of Conduct

@rly rly added the category: bug errors in the code or code behavior label Nov 22, 2024
@rly
Copy link
Contributor Author

rly commented Nov 22, 2024

I propose something like:

def _raise_pynwb_import_error():
    raise ImportError("pynwb is not installed. Please install pynwb to use NWBZarrIO.")

try:
    from pynwb import ...

    class NWBZarrIO(ZarrIO):
        # copy here
        pass

except ImportError:
    NWBZarrIO = _raise_pynwb_import_error

Related: NeurodataWithoutBorders/pynwb#1977

@rly rly changed the title [Bug]: Refactor try/except in nwb.py [Bug]: Refactor import error handling in nwb.py Nov 22, 2024
@rly rly changed the title [Bug]: Refactor import error handling in nwb.py [Bug]: Error handling in nwb.py masks true error Nov 22, 2024
@rly
Copy link
Contributor Author

rly commented Nov 22, 2024

pynwb is currently a requirement of hdmf-zarr so this try/except is not even needed

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

Successfully merging a pull request may close this issue.

1 participant