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

Certain versions of numpy & cython cause problems #1538

Closed
ululi1970 opened this issue Apr 22, 2020 · 9 comments · Fixed by #1611
Closed

Certain versions of numpy & cython cause problems #1538

ululi1970 opened this issue Apr 22, 2020 · 9 comments · Fixed by #1611

Comments

@ululi1970
Copy link

To assist reproducing bugs, please include the following:

  • Operating System (e.g. Windows 10, MacOS 10.11, Ubuntu 16.04.2 LTS, CentOS 7)
    RedHat
  • Python version (e.g. 2.7, 3.5)
    3.6
  • Where Python was acquired (e.g. system Python on MacOS or Linux, Anaconda on
    Windows)
    system python
  • h5py version (e.g. 2.6)
    latest from branch
  • HDF5 version (e.g. 1.8.17)
    1.10.6
  • The full traceback/stack trace shown (if it appears)
    File "/nas/longleaf/home/XXX/.local/lib/python3.6/site-packages/h5py/init.py",
    line 45, in
    from ._conv import register_converters as _register_converters,
    File "h5py/h5t.pxd", line 15, in init h5py._conv
    File "h5py/h5t.pyx", line 262, in init h5py.h5t
    File "h5py/h5t.pyx", line 257, in h5py.h5t._get_available_ftypes
    AttributeError: 'numpy.dtype' object has no attribute 'typeobj'

h5py.version.info
Not available. Traceback given above.
However, this error occurs only when compiling from sources pulled from the master branch on github. pip install --no-binary=h5py h5py works fine.

This error occurs when the available version of numpy<=1.17. It works with numpy>=1.18.3
I suggest to update the requirements. Right now, it checks for numpy >=1.7.

@takluyver
Copy link
Member

I'm puzzled by this, because we should be testing back to numpy version 1.12, depending on the Python version (see tox.ini, specifically the mindeps entries). And the CI is passing. Is it possible that the coincidence of an older numpy and a newer Cython causes the problem?

If there is an issue, then apart from the need to fix our testing, I think we should try to find a fix before bumping the minimum version of numpy. If we follow NEP 29 as a guideline, we wouldn't drop numpy 1.17 until July 2021.

@ululi1970
Copy link
Author

ululi1970 commented Apr 23, 2020 via email

@takluyver
Copy link
Member

In fact, this seems to have come up before - #1436 - but the person who reported it then couldn't reproduce it. Can you check what version of cython you have installed where it fails with an older numpy version? You should be able to see this by importing it and checking cython.__version__.

#1439 was meant to fix that, but stalled because no-one knew how to reproduce the bug. If you can verify that that change applied to master does solve the problem, it should be safe to merge.

I wonder if there's a combination we can add on CI that would have caught this, though.

@ululi1970
Copy link
Author

ululi1970 commented Apr 23, 2020 via email

@tacaswell
Copy link
Member

It looks like there is a window of numpy versions that don't work. So far I have

cython version np version works
0.29.16 1.12.0 ✔️
0.28.5 1.17.2
0.28.5 1.17.1
0.28.5 1.16.6
0.28.5 1.15.4
0.28.5 1.14.6
0.28.5 1.13.3
0.28.5 1.12.1
0.29.16 1.12.1 ✔️

Which now that I have done that search against numpy, I suspect the version that actually matters is the cython version or there is some relationship between the two. I was doing this by hand, I suspect we need to write a script to do a brute force search of the versions....

@takluyver takluyver changed the title need to upgrade numpy version requirements to 1.18 Certain versions of numpy & cython cause problems Jul 31, 2020
@tacaswell
Copy link
Member

The culprit here is the cython version, not the numpy version. I suspect this is due to a change in 252e3bf to how we import numpy into our cython modules.

cython works
0.23 ✔️
0.23.1 ✔️
0.23.2 ✔️
0.23.3 ✔️
0.23.4 ✔️
0.23.5 ✔️
0.24 ✔️
0.24.1 ✔️
0.25
0.25.1
0.25.2
0.26
0.26.1
0.27
0.27.1
0.27.2
0.27.3
0.28
0.28.1
0.28.2
0.28.3
0.28.4
0.28.5
0.28.6
0.29 ✔️
0.29.1 ✔️
0.29.2 ✔️
0.29.3 ✔️
0.29.4 ✔️
0.29.5 ✔️
0.29.6 ✔️
0.29.7 ✔️
0.29.8 ✔️
0.29.9 ✔️
0.29.10 ✔️
0.29.11 ✔️
0.29.12 ✔️
0.29.13 ✔️
0.29.14 ✔️
0.29.15 ✔️
0.29.16 ✔️
0.29.17 ✔️
0.29.18 ✔️
0.29.19 ✔️
0.29.20 ✔️
0.29.21 ✔️

I think that simplest "fix" is to bad known-bad cython or require >=0.29. We already require (in the tests at least and I suspect practically due do the cython support for cpython windows) >= 0.29 for py37 and py38.

@tacaswell
Copy link
Member

On slightly deeper inspection, 2aabf9d bumped the minimum cython to 0.25 as part of the change to how we bind numpy (,#1390) (but it seems to build and pass tests on current master with cython 0.24.1?).

Cython 0.29 was released in Oct 2018. Debian buster (which is the oldest debian that ships a Python we support (stretch (aka oldstable) is py35, buster (aka stable) is py37) has cython 0.29.

fedora30 and above have cython 0.29, RHEL/centos7 does not ship a Python we support, centos/rhel 8 has 0.29.

tacaswell added a commit to tacaswell/h5py that referenced this issue Aug 1, 2020
@aragilar
Copy link
Member

aragilar commented Aug 2, 2020

Did we want to do the same "last 2 years of releases" policy for cython as numpy (I suspect that for newer python versions we may need a more recent release of cython, but we probably want to keep moving up the lower bound anyway)?

@takluyver
Copy link
Member

I'm OK with requiring newer versions of Cython, as a build dependency, than we'd accept for runtime dependencies like Python & numpy. Even if someone wants to build a new h5py on an older Linux distro, it should be possible for them to install a recent version of Cython in a virtualenv and use that to build a compatible h5py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants