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

Fails to build with h5py 2.10.0 #343

Closed
tillea opened this issue May 7, 2020 · 5 comments
Closed

Fails to build with h5py 2.10.0 #343

tillea opened this issue May 7, 2020 · 5 comments
Labels
category: bug errors in the code or code behavior

Comments

@tillea
Copy link

tillea commented May 7, 2020

Hi,
there is a bug report against the Debian packaged hdmf version 1.5.4. I tried the latest version 1.6.1 which shows the same issue. It would be great if you could adapt hdmf to the latest h5py version.
Kind regards, Andreas.

@tillea tillea added the category: bug errors in the code or code behavior label May 7, 2020
@rly
Copy link
Contributor

rly commented May 8, 2020

Hi @tillea . Thanks for the bug report. Unfortunately, I cannot reproduce this on my system with hdmf version 1.5.4 and this will require more investigation. From the bug report, I think the issue is with the h5py build on debian. As a test, would you be able to create test.py:

import h5py
import unittest 
test = unittest.TestCase()
with test.assertWarns(Warning):
    print('test')

and run python test.py. Does this result in an error?

My personal debugging notes on the error:
Errors occur in multiple tests on the usage of unittest.assertWarns. The unittest.assertWarns context manager looks for the __warning_registry__ attribute on every module in sys.modules (current source: https://github.com/python/cpython/blob/master/Lib/unittest/case.py#L256). This triggers legacy code h5py_warnings.ModuleWrapper.__getattr__ in h5py 2.10.0 that tries to import a module if it is not imported (this code was removed after 2.10.0 in h5py/h5py#1370). Importing a module in import_module calls the startswith function on the module, which begins an infinite loop back to h5py_warnings.ModuleWrapper.__getattr__. The h5py_warnings.ModuleWrapper class was added to wrap the deprecated h5py.highlevel API in h5py.__init__. See h5py/h5py#1046 and PR 1047. So, why is that module in the warning registry?

This might be solved by importing h5py.highlevel outside of the tests, but this module is deprecated. It looks like PyMVPA may have had a similar issue where the build broke and importing it fixed it: PyMVPA/PyMVPA@f0e5513

Another workaround might be to clear the warning registry for h5py.highlevel using something like numpy.testing.clear_and_catch_warnings (https://www.kite.com/python/docs/numpy.testing.clear_and_catch_warnings)

Speculation: It could be that something went awry during the port of h5py 2.10.0-7. The source code for h5py 2.10.0-7 includes a patch that removes references to the h5py.highlevel package and removes the h5py_warnings.ModuleWrapper class. ModuleWrapper is in the error stack trace, but how, if this patch is applied? Also, h5py_2.10.0-7.debian/debian/wrapper_module/h5py/__init__.py includes code that changes sys.modules. It might accidentally include h5py.highlevel.

@AdrianBunk
Copy link

As a test, would you be able to create test.py:

import h5py
import unittest 
test = unittest.TestCase()
with test.assertWarns(Warning):
    print('test')

and run python test.py. Does this result in an error?

Yes, this reproduces the problem.

Speculation: It could be that something went awry during the port of h5py 2.10.0-7. The source code for h5py 2.10.0-7 includes a patch that removes references to the h5py.highlevel package and removes the h5py_warnings.ModuleWrapper class. ModuleWrapper is in the error stack trace, but how, if this patch is applied?

remove_deprecated_highlevel_module_2f41c78.patch is not applied:
https://sources.debian.org/src/h5py/2.10.0-7/debian/patches/series/

@yarikoptic
Copy link
Contributor

yarikoptic commented May 8, 2020

import h5py
import unittest 
test = unittest.TestCase()
with test.assertWarns(Warning):
    print('test')
FWIW, this snippet does lead to infinite recursion with debian 2.10.0-7 and numpy 1.16.5:
$> python3 test_it.py 2>&1 | head -n 20
Traceback (most recent call last):
  File "test_it.py", line 4, in <module>
    with test.assertWarns(Warning):
  File "/usr/lib/python3.8/unittest/case.py", line 255, in __enter__
    if getattr(v, '__warningregistry__', None):
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 36, in __getattr__
    return getattr(self._mod, attr)
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 35, in __getattr__
    self._mod = self._import()
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 39, in _import
    return import_module(self._mod)
  File "/usr/lib/python3.8/importlib/__init__.py", line 118, in import_module
    if name.startswith('.'):
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 35, in __getattr__
    self._mod = self._import()
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 39, in _import
    return import_module(self._mod)
  File "/usr/lib/python3.8/importlib/__init__.py", line 118, in import_module
    if name.startswith('.'):
  File "/usr/lib/python3/dist-packages/h5py/_debian_h5py_serial/h5py_warnings.py", line 35, in __getattr__
force-installing in a virtualenv stock h5py 2.10.0 (while allowing for system site packages) which pulls in newer numpy and six results in expected failure to see the warning on print:
$> py=3; d=venvs/dev$py; virtualenv --python=python$py --system-site-packages $d && source $d/bin/activate && pip3 install --upgrade --force-reinstall h5py 
created virtual environment CPython3.8.3.candidate.1-64 in 160ms
  creator CPython3Posix(dest=/tmp/test-hdmf/venvs/dev3, clear=False, global=True)
  seeder FromAppData(download=False, chardet=latest, progress=latest, distlib=latest, contextlib2=latest, colorama=latest, appdirs=latest, pip=latest, urllib3=latest, CacheControl=latest, setuptools=latest, lockfile=latest, distro=latest, pkg_resources=latest, pytoml=latest, idna=latest, html5lib=latest, wheel=latest, pyparsing=latest, msgpack=latest, certifi=latest, webencodings=latest, pep517=latest, packaging=latest, six=latest, retrying=latest, requests=latest, via=copy, app_data_dir=/home/yoh/.local/share/virtualenv/seed-app-data/v1.0.1.debian)
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
Collecting h5py
  Using cached h5py-2.10.0-cp38-cp38-manylinux1_x86_64.whl (2.9 MB)
Collecting six
  Using cached six-1.14.0-py2.py3-none-any.whl (10 kB)
Collecting numpy>=1.7
  Using cached numpy-1.18.4-cp38-cp38-manylinux1_x86_64.whl (20.7 MB)
ERROR: launchpadlib 1.10.13 requires testresources, which is not installed.
Installing collected packages: six, numpy, h5py
  Attempting uninstall: six
    Found existing installation: six 1.14.0
    Uninstalling six-1.14.0:
      Successfully uninstalled six-1.14.0
  Attempting uninstall: numpy
    Found existing installation: numpy 1.17.4
    Not uninstalling numpy at /usr/lib/python3/dist-packages, outside environment /tmp/test-hdmf/venvs/dev3
    Can't uninstall 'numpy'. No files were found to uninstall.
  Attempting uninstall: h5py
    Found existing installation: h5py 2.10.0
    Not uninstalling h5py at /usr/lib/python3/dist-packages, outside environment /tmp/test-hdmf/venvs/dev3
    Can't uninstall 'h5py'. No files were found to uninstall.
Successfully installed h5py-2.10.0 numpy-1.18.4 six-1.14.0

$> python3 test_it.py 2>&1                                                                                                                                 
test
Traceback (most recent call last):
  File "test_it.py", line 5, in <module>
    print('test')
  File "/usr/lib/python3.8/unittest/case.py", line 294, in __exit__
    self._raiseFailure("{} not triggered".format(exc_name))
  File "/usr/lib/python3.8/unittest/case.py", line 164, in _raiseFailure
    raise self.test_case.failureException(msg)
AssertionError: Warning not triggered

edit1: force installing newer numpy and six only still leads to the same infinite recursion, so the issue seems indeed caused h5py debian port

@rly
Copy link
Contributor

rly commented May 8, 2020

@tillea @yarikoptic the debian port contains this code in h5py_2.10.0-7.debian/debian/wrapper_module/h5py/__init__.py

from sys import modules as sys_modules

# [snip]

# make generic h5py module behaviour the same as specific builds
# by importing public and weak internal symbols (single _underscore)
api = [ k for k in _h5py.__dict__.keys() if not k.startswith('__') and not k.endswith('__') ]
this_module=sys_modules[__name__]
for key in api:
    # "imports" symbols (makes them accessible)
    setattr(this_module,key,getattr(_h5py,key))
    # rename symbols as properties of toplevel h5py module
    sys_modules['h5py.{}'.format(key)] = getattr(_h5py,key)

Since remove_deprecated_highlevel_module_2f41c78.patch is not applied, the api list includes 'highlevel' so then sys.modules['h5py.highlevel'] is set to h5py.highlevel. This is problematic because sys.modules is traversed in the context manager for unittest.TestCase.assertWarns and getattr is called on it, but h5py.highlevel is intentionally lazily imported by h5py, I think, because it is deprecated. So one solution might be to apply the patch. Another might be to add and not k == 'highlevel' to the line that sets api above.

In any case, this is an issue with h5py on debian, not hdmf, so I will close this.

@rly rly closed this as completed May 8, 2020
@yarikoptic
Copy link
Contributor

Thank you @rly , further communication will continue in that debian bug report @tillea has mentioned: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=959940

Cheers!

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

4 participants