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]: AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links' #817

Closed
3 tasks done
yarikoptic opened this issue Jan 18, 2023 · 22 comments · Fixed by #818
Closed
3 tasks done

[Bug]: AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links' #817

yarikoptic opened this issue Jan 18, 2023 · 22 comments · Fixed by #818
Labels
category: bug errors in the code or code behavior priority: critical impacts proper operation or use of core function of NWB or the software

Comments

@yarikoptic
Copy link
Contributor

What happened?

@TheChymera managed to get our CIs all (seems happens across OSes) fail out with

FAILED dandi/tests/test_files.py::test_validate_bogus - pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7f42d37f4550>

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/hdmf/backends/io.py", line 137, in __del__
    self.close()
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 738, in close
    self.close_linked_files()
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 749, in close_linked_files
    for obj in self.__open_links:
AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'

which you can observe in https://github.com/dandi/dandi-cli/actions/runs/3952213342/jobs/6767014502 . What could this be due to???

Steps to Reproduce

- Get born to be @TheChymera
- Send a PR

Traceback

above

Operating System

Linux

Python Executable

Conda

Python Version

3.10

Package Versions

2023-01-18T19:23:49.7460559Z Successfully installed Deprecated-1.2.13 SecretStorage-3.3.3 anys-0.2.1 appdirs-1.4.4 arrow-1.2.3 asciitree-0.3.3 attrs-22.2.0 bidsschematools-0.6.0 blessings-1.7 certifi-2022.12.7 cffi-1.15.1 charset-normalizer-3.0.1 ci-info-0.3.0 click-8.1.3 click-didyoumean-0.3.0 coverage-7.0.5 cryptography-39.0.0 dandi-0.48.1+12.g65368a6 dandischema-0.7.1 dnspython-2.3.0 email-validator-1.3.0 entrypoints-0.4 etelemetry-0.3.0 exceptiongroup-1.1.0 fasteners-0.18 fqdn-1.5.1 fscacher-0.2.0 h5py-3.7.0 hdmf-3.5.0 humanize-4.4.0 idna-3.4 importlib-metadata-6.0.0 iniconfig-2.0.0 interleave-0.2.1 isodate-0.6.1 isoduration-20.11.0 jaraco.classes-3.2.3 jeepney-0.8.0 joblib-1.2.0 jsonpointer-2.3 jsonschema-4.17.3 keyring-23.13.1 keyrings.alt-4.2.0 more-itertools-9.0.0 natsort-8.2.0 numcodecs-0.11.0 numpy-1.23.5 nwbinspector-0.4.25 opencv-python-4.7.0.68 packaging-23.0 pandas-1.5.2 pluggy-1.0.0 pycparser-2.21 pycryptodomex-3.16.0 pydantic-1.10.4 pynwb-2.2.0 pyout-0.7.2 pyrsistent-0.19.3 pytest-7.2.1 pytest-cov-4.0.0 pytest-mock-3.10.0 python-dateutil-2.8.2 pytz-2022.7.1 pyyaml-6.0 requests-2.28.2 responses-0.22.0 rfc3339-validator-0.1.4 rfc3987-1.3.8 ruamel.yaml-0.17.21 ruamel.yaml.clib-0.2.7 scipy-1.10.0 semantic-version-2.10.0 six-1.16.0 tenacity-8.1.0 toml-0.10.2 tomli-2.0.1 tqdm-4.64.1 types-toml-0.10.8.1 typing-extensions-4.4.0 uri-template-1.2.0 urllib3-1.26.14 webcolors-1.12 wrapt-1.14.1 zarr-2.13.6 zipp-3.11.0

Code of Conduct

@oruebel oruebel added category: bug errors in the code or code behavior priority: critical impacts proper operation or use of core function of NWB or the software labels Jan 18, 2023
@oruebel oruebel added this to the Next Release milestone Jan 18, 2023
@oruebel
Copy link
Contributor

oruebel commented Jan 18, 2023

Based on the error, I assume this bug is triggered by the changes from this PR https://github.com/hdmf-dev/hdmf/pull/811/files That PR added the __del__ method to ensure close is being called any time anHDMFIO object (here NWBHDF5IO) is being destroyed. The part that is confusing to me is that __open_links is being set in the constructor of HDF5IO and I don't yet understand why it ever would not be present. A simple fix would be to modify the close_linked_file function to check whether __open_links exists before trying to access it (or have a try/except for the AttributeError). However, I still would really like to understand under what circumstances this is error is happening as I currently can't reproduce it on my system. Do you have a simple script that triggers this error so that I can retrace what is happening and also add a unit test for this.

def close_linked_files(self):
"""Close all opened, linked-to files.
MacOS and Linux automatically release the linked-to file after the linking file is closed, but Windows does
not, which prevents the linked-to file from being deleted or truncated. Use this method to close all opened,
linked-to files.
"""
for obj in self.__open_links:
if obj:
obj.file.close()
self.__open_links = []

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

@yarikoptic could you test to see if #818 fixes the issue. Also if you have a short script to reproduce the problem would be great as I'm not sure right now how to reproduce the issue and I would really like to better understand what triggers this issue.

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

I guess this could happen if an error occurs during HDF5IO.__init__ such that del would be called on an object that has not been fully initialized

@rly
Copy link
Contributor

rly commented Jan 19, 2023

I think Oliver's hunch is correct:

>>> io = pynwb.NWBHDF5IO(path=None, file=None, mode="r")
Exception ignored in: <function HDMFIO.__del__ at 0x11c660790>
Traceback (most recent call last):
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/io.py", line 137, in __del__
    self.close()
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 738, in close
    self.close_linked_files()
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 749, in close_linked_files
    for obj in self.__open_links:
AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/Users/rly/Documents/NWB/pynwb/src/pynwb/__init__.py", line 242, in __init__
    super().__init__(path, manager=manager, mode=mode, file=file_obj, comm=comm, driver=driver)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 59, in __init__
    raise ValueError("You must supply either a path or a file.")
ValueError: You must supply either a path or a file.

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

A similar error can technically also happen in HDF5IO.close when HDF5IO.__file is missing.

if self.__file is not None:
self.__file.close()

I'll add a fix in #818 for this case well

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

I believe #818 should fix this case. Please have a look and let me know what you think.

@TheChymera
Copy link

So I also tested this with different versions, and apparently the issue shows up with =hdmf-3.5.0 but not with =hdmf-3.4.7.

Sadly, it also still happens after applying the proposed patch :(

(dev) [deco]~/src/dandi-cli ❱ pytest -vvs dandi/tests/test_files.py::test_validate_bogus
=========================================================== test session starts ============================================================
platform linux -- Python 3.10.9, pytest-7.2.1, pluggy-1.0.0 -- /usr/bin/python3.10
cachedir: .pytest_cache
rootdir: /home/chymera/src/dandi-cli, configfile: tox.ini
plugins: pkgcore-0.12.18, mock-3.10.0
collected 1 item

dandi/tests/test_files.py::test_validate_bogus FAILED

================================================================= FAILURES =================================================================
___________________________________________________________ test_validate_bogus ____________________________________________________________
/usr/lib/python3.10/site-packages/_pytest/runner.py:339: in from_call
    result: Optional[TResult] = func()
/usr/lib/python3.10/site-packages/_pytest/runner.py:260: in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
/usr/lib/python3.10/site-packages/pluggy/_hooks.py:265: in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
/usr/lib/python3.10/site-packages/pluggy/_manager.py:80: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
/usr/lib/python3.10/site-packages/_pytest/unraisableexception.py:88: in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
/usr/lib/python3.10/site-packages/_pytest/unraisableexception.py:78: in unraisable_exception_runtest_hook
    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7fbdd2b115a0>
E
E   Traceback (most recent call last):
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 758, in close_linked_files
E       for obj in self.__open_links:
E   AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
E
E   During handling of the above exception, another exception occurred:
E
E   Traceback (most recent call last):
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/io.py", line 137, in __del__
E       self.close()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 738, in close
E       self.close_linked_files()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 766, in close_linked_files
E       warnings.warn("HDF5IO was not fully initialized before close. Missing self.__open_links.")
E   UserWarning: HDF5IO was not fully initialized before close. Missing self.__open_links.
=========================================================== slowest 10 durations ===========================================================
0.08s call     dandi/tests/test_files.py::test_validate_bogus
0.00s setup    dandi/tests/test_files.py::test_validate_bogus
0.00s teardown dandi/tests/test_files.py::test_validate_bogus
========================================================= short test summary info ==========================================================
FAILED dandi/tests/test_files.py::test_validate_bogus - pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7fbdd2b115a0>
============================================================ 1 failed in 0.23s =============================================================

@TheChymera
Copy link

Here's the diff between the failure before and after the patch:

(dev) [deco]~/src/dandi-cli ❱ colordiff /tmp/fail /tmp/failpatch
24c24,31
< E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7f76dc3795a0>
---
> E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7fec459c55a0>
> E
> E   Traceback (most recent call last):
> E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 758, in close_linked_files
> E       for obj in self.__open_links:
> E   AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
> E
> E   During handling of the above exception, another exception occurred:
31,33c38,40
< E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 749, in close_linked_files
< E       for obj in self.__open_links:
< E   AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
---
> E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 766, in close_linked_files
> E       warnings.warn("HDF5IO was not fully initialized before close. Missing self.__open_links.")
> E   UserWarning: HDF5IO was not fully initialized before close. Missing self.__open_links.
40c47
< ============================== 1 failed in 0.19s ===============================
---
> ============================== 1 failed in 0.18s ===============================

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

So I also tested this with different versions, and apparently the issue shows up with =hdmf-3.5.0 but not with =hdmf-3.4.7.

Thanks for checking this. That matches what I suspected. The changes from #811 were released in 3.5

Sadly, it also still happens after applying the proposed patch :(

Based on the error, I suspect that the test may inadvertently not actually have used the patched version. You probably will need to install PyNWB first, and then uninstall HDMF and reinstall HDMF from the patched branch. If you install HDMF first and then pip install PyNWB, I think it may possibly replace your HDMF install with the 3.5 version from PyPI.

@rly
Copy link
Contributor

rly commented Jan 19, 2023

@oruebel the new warning that you added in #818 showed up in the above stack trace, so I think they were using the patched version.

The failing test appears to be dandi/tests/test_files.py::test_validate_bogus:
https://github.com/dandi/dandi-cli/blob/54e2a123d7f36dfe7cf2d8a4e69806070bc5fac8/dandi/tests/test_files.py#L355

Is that correct? It seems like the test is trying to read and/or validate a file that is not an NWB file. I haven't been able to reproduce the error writing my own simplified version of that test though.

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

@oruebel the new warning that you added in #818 showed up in the above stack trace, so I think they were using the patched version.

Yes, I stand correct. Sorry, based on your diff of the error it looks like it is indeed using the branch. If I read the diff correctly, then with HDFM 3.5 you saw an AttributeError and with the patch you are seeing a UserWarning instead. That behavior is expected. If the test is failing because of the warning, then maybe the test could be updated to check for the warning. If raising a warning is an issue, then we could also silently accept the AttributeError. I don't think that should be an issue, I just figured raising the warning would be helpful as additional context for debugging for uses and developers.

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

@rly a simple test for this is

from pynwb import NWBHDF5IO

filename = 'bad_nwb_file.nwb'
with open(filename, 'w') as f:
    f.write("I am a text file pretending to be an NWB file")

with NWBHDF5IO(filename, 'r') as io:
    io.read()

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

@TheChymera can you run the test again. I moved the initalization of the __open_links and __file to the beginning in HDFIO.__initi__ which I think should prevent the error from occurring for NWBHDF5IO.

f79b3a2

@rly
Copy link
Contributor

rly commented Jan 19, 2023

Oh I see. In the Python interpreter, I just get OSError: Unable to open file (file signature not found). But when I exit the Python interpreter, I guess garbage collection runs which calls __del__ and I get the warning. I don't get the original error though - perhaps that shows up only using pytest

Python 3.10.8 | packaged by conda-forge | (main, Nov 22 2022, 08:25:29) [Clang 14.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pynwb import NWBHDF5IO
>>> 
>>> filename = 'bad_nwb_file.nwb'
>>> with open(filename, 'w') as f:
...     f.write("I am a text file pretending to be an NWB file")
... 
45
>>> with NWBHDF5IO(filename, 'r') as io:
...     io.read()
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/Users/rly/Documents/NWB/pynwb/src/pynwb/__init__.py", line 242, in __init__
    super().__init__(path, manager=manager, mode=mode, file=file_obj, comm=comm, driver=driver)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 85, in __init__
    super().__init__(manager, source=path)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/io.py", line 23, in __init__
    self.open()
  File "/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 729, in open
    self.__file = File(self.source, open_flag, **kwargs)
  File "/Users/rly/mambaforge/envs/dev/lib/python3.10/site-packages/h5py/_hl/files.py", line 533, in __init__
    fid = make_fid(name, mode, userblock_size, fapl, fcpl, swmr=swmr)
  File "/Users/rly/mambaforge/envs/dev/lib/python3.10/site-packages/h5py/_hl/files.py", line 226, in make_fid
    fid = h5f.open(name, flags, fapl=fapl)
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "h5py/h5f.pyx", line 106, in h5py.h5f.open
OSError: Unable to open file (file signature not found)
>>> exit()
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:766: UserWarning: HDF5IO was not fully initialized before close. Missing self.__open_links.
  warnings.warn("HDF5IO was not fully initialized before close. Missing self.__open_links.")

@oruebel
Copy link
Contributor

oruebel commented Jan 19, 2023

. I don't get the original error though - perhaps that shows up only using pytest

I think the only time you should see the original error is if you add a warnings filter to elevate UserWarnings to errors. Depending on how pytest is being run, I think it may treat any uncaught warnings as errors. Otherwise, you should only the the warning.

I think with f79b3a2 the error should also no longer occure for NWBHDF5IO. With this change, the only time the error should happen if an error is raised in the child class of HDF5IO before HDF5IO._init has been called.

@TheChymera
Copy link

TheChymera commented Jan 20, 2023

@oruebel @rly

Thanks so much for your help in sorting this out... however, it still fails:

(dev) [deco]~/src/dandi-cli ❱ pytest -vvs dandi/tests/test_files.py::test_validate_bogus
=========================================================== test session starts ============================================================
platform linux -- Python 3.10.9, pytest-7.2.1, pluggy-1.0.0 -- /usr/bin/python3.10
cachedir: .pytest_cache
rootdir: /home/chymera/src/dandi-cli, configfile: tox.ini
plugins: pkgcore-0.12.18, mock-3.10.0
collected 1 item

dandi/tests/test_files.py::test_validate_bogus FAILED

================================================================= FAILURES =================================================================
___________________________________________________________ test_validate_bogus ____________________________________________________________
/usr/lib/python3.10/site-packages/_pytest/runner.py:339: in from_call
    result: Optional[TResult] = func()
/usr/lib/python3.10/site-packages/_pytest/runner.py:260: in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
/usr/lib/python3.10/site-packages/pluggy/_hooks.py:265: in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
/usr/lib/python3.10/site-packages/pluggy/_manager.py:80: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
/usr/lib/python3.10/site-packages/_pytest/unraisableexception.py:88: in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
/usr/lib/python3.10/site-packages/_pytest/unraisableexception.py:78: in unraisable_exception_runtest_hook
    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7f1f561d15a0>
E
E   Traceback (most recent call last):
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 760, in close_linked_files
E       for obj in self.__open_links:
E   AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
E
E   During handling of the above exception, another exception occurred:
E
E   Traceback (most recent call last):
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/io.py", line 137, in __del__
E       self.close()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 740, in close
E       self.close_linked_files()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 768, in close_linked_files
E       warnings.warn("HDF5IO was not fully initialized before close. Missing self.__open_links.")
E   UserWarning: HDF5IO was not fully initialized before close. Missing self.__open_links.
=========================================================== slowest 10 durations ===========================================================
0.08s call     dandi/tests/test_files.py::test_validate_bogus
0.00s setup    dandi/tests/test_files.py::test_validate_bogus
0.00s teardown dandi/tests/test_files.py::test_validate_bogus
========================================================= short test summary info ==========================================================
FAILED dandi/tests/test_files.py::test_validate_bogus - pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7f1f561d15a0>
============================================================ 1 failed in 0.23s =============================================================
(dev) [deco]~/src/dandi-cli ❱ pytest -vvs dandi/tests/test_files.py::test_validate_bogus &> /tmp/failpatch2
(dev) [deco]~/src/dandi-cli ❱ diff /tmp/failpatch /tmp/failpatch2
24c24
< E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7fec459c55a0>
---
> E   pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function HDMFIO.__del__ at 0x7f976d7b95a0>
27c27
< E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 758, in close_linked_files
---
> E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 760, in close_linked_files
36c36
< E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 738, in close
---
> E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 740, in close
38c38
< E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 766, in close_linked_files
---
> E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 768, in close_linked_files
47c47
< ============================== 1 failed in 0.18s ===============================
---
> ============================== 1 failed in 0.19s ===============================
(dev) [deco]~/src/dandi-cli ❱ head -60 /usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py | tail -4

        self.__open_links = []  # keep track of other files opened from links in this file
        self.__file = None  # This will be set below, but set to None first in case an error occurs and we need to close

Last bit of the code showcasing that it's indeed the patch for the entire PR, this part being what f79b3a2 adds.

@TheChymera
Copy link

TheChymera commented Jan 20, 2023

@oruebel

If I read the diff correctly, then with HDFM 3.5 you saw an AttributeError and with the patch you are seeing a UserWarning instead. That behavior is expected. If the test is failing because of the warning, then maybe the test could be updated to check for the warning. If raising a warning is an issue, then we could also silently accept the AttributeError.

Well, I actually get both the AttributeError and the UserWarning, hopefully the traceback in the post above presents the full picture.

Also, afaict, I get the same traceback from the example test as @rly mentioned above.

[deco]/tmp/hdmf_bug ❱ python lala.py
Traceback (most recent call last):
  File "/tmp/hdmf_bug/lala.py", line 7, in <module>
    with NWBHDF5IO(filename, 'r') as io:
  File "/usr/lib/python3.10/site-packages/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/usr/lib/python3.10/site-packages/pynwb/__init__.py", line 234, in __init__
    super().__init__(path, manager=manager, mode=mode, file=file_obj, comm=comm, driver=driver)
  File "/usr/lib/python3.10/site-packages/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 88, in __init__
    super().__init__(manager, source=path)
  File "/usr/lib/python3.10/site-packages/hdmf/utils.py", line 645, in func_call
    return func(args[0], **pargs)
  File "/usr/lib/python3.10/site-packages/hdmf/backends/io.py", line 23, in __init__
    self.open()
  File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 731, in open
    self.__file = File(self.source, open_flag, **kwargs)
  File "/usr/lib/python3.10/site-packages/h5py/_hl/files.py", line 533, in __init__
    fid = make_fid(name, mode, userblock_size, fapl, fcpl, swmr=swmr)
  File "/usr/lib/python3.10/site-packages/h5py/_hl/files.py", line 226, in make_fid
    fid = h5f.open(name, flags, fapl=fapl)
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "h5py/h5f.pyx", line 106, in h5py.h5f.open
OSError: Unable to open file (file signature not found)
[deco]/tmp/hdmf_bug ❱ cat lala.py
from pynwb import NWBHDF5IO

filename = 'bad_nwb_file.nwb'
with open(filename, 'w') as f:
    f.write("I am a text file pretending to be an NWB file")

with NWBHDF5IO(filename, 'r') as io:
    io.read()

But this is an OSError, so I'm not sure whether I can use this MWE to determine whether pytest is messing something up.

It seems pytest doesn't like to be used for MWEs 🤔

[deco]/tmp/hdmf_bug ❱ pytest -vvs tests/test_lala.py
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --doctest-cython
  inifile: /tmp/pytest.ini
  rootdir: /tmp

[deco]/tmp/hdmf_bug ❱ cat tests/test_lala.py
from pynwb import NWBHDF5IO

test_lulu():
    filename = 'bad_nwb_file.nwb'
    with open(filename, 'w') as f:
        f.write("I am a text file pretending to be an NWB file")

    with NWBHDF5IO(filename, 'r') as io:
        io.read()

@oruebel
Copy link
Contributor

oruebel commented Jan 20, 2023

E   AttributeError: 'NWBHDF5IO' object has no attribute '_HDF5IO__open_links'
E
E   During handling of the above exception, another exception occurred:
E
E   Traceback (most recent call last):
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/io.py", line 137, in __del__
E       self.close()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 740, in close
E       self.close_linked_files()
E     File "/usr/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py", line 768, in close_linked_files
E       warnings.warn("HDF5IO was not fully initialized before close. Missing self.__open_links.")
E   UserWarning: HDF5IO was not fully initialized before close. Missing self.__open_links.

As far as I understand this part of the error, this means that pytest sees the UserWarning and instead of treating it like a warning it elevates all warnings to an exception. I would suggest updating the test-case to appropriately catch or ignore the warning, for example, by wrapping the call where the warning occurs via a with warnings.catch_warnings(): context, e.g.,:

def test_validate_bogus(tmp_path):
    path = tmp_path / "wannabe.nwb"
    path.write_text("not really nwb")
    # intended to produce use-case for https://github.com/dandi/dandi-cli/issues/93
    # but it would be tricky, so it is more of a smoke test that
    # we do not crash
    with warnings.catch_warnings():
          warnings.filterwarnings(''ignore', message='HDF5IO was not fully initialized before close')
          errors = dandi_file(path).get_validation_errors()
    # ATM we would get 2 errors -- since could not be open in two places,
    # but that would be too rigid to test. Let's just see that we have expected errors
    assert any(e.message.startswith("Unable to open file") for e in errors)

Alternatively, we could also just remove the warning altogether, but personally I think it is helpful to have as additional context for debugging for uses and developers.

TheChymera added a commit to dandi/dandi-cli that referenced this issue Jan 23, 2023
@TheChymera
Copy link

@oruebel thank you for your help, if you strongly prefer to raise the warning, we can indeed adapt, as you said :)

Would it be possible to have a patch release (3.5.1) already, or would this one issue seem too minor for that?

@oruebel
Copy link
Contributor

oruebel commented Jan 23, 2023

Doing a 3.5.1 release for this seems fine. @rly what do you think about raising the warning or silently catching the error?

@rly
Copy link
Contributor

rly commented Jan 24, 2023

On further thought, I am in favor of silently catching the fact that a variable was not initialized (do not raise a warning). If I understand correctly, this warning would only be raised if an error interrupted execution of the HDF5IO constructor. The user will be alerted to something unusual happening from the error. The close and __del__ methods would run as expected, closing the file or links if they have been opened. @oruebel can you update the PR to remove the warning? Thanks.

@oruebel
Copy link
Contributor

oruebel commented Jan 24, 2023

can you update the PR to remove the warning?

Done

oruebel added a commit that referenced this issue Jan 24, 2023
…818)

* Fix #817 Check that __open_links exists before trying to close links
* Catch possible missing HDF5IO.__file error
* Add unit test for case where HDF5IO.close is called before HDF5IO.__init__ is complete
* Move init of __file and __openlink up to prevent warning during close
* Update changelog

Co-authored-by: Ryan Ly <[email protected]>
@rly rly removed this from the 3.14.0 milestone Apr 11, 2024
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 priority: critical impacts proper operation or use of core function of NWB or the software
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants