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

Error importing cyvcf2 with numpy 2.0.0 #307

Closed
lacek opened this issue Jun 17, 2024 · 10 comments
Closed

Error importing cyvcf2 with numpy 2.0.0 #307

lacek opened this issue Jun 17, 2024 · 10 comments

Comments

@lacek
Copy link

lacek commented Jun 17, 2024

Got the following error when importing cyvcf2:

$ python3
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import cyvcf2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/dist-packages/cyvcf2/__init__.py", line 1, in <module>
    from .cyvcf2 import (VCF, Variant, Writer, r_ as r_unphased, par_relatedness,
  File "cyvcf2/cyvcf2.pyx", line 1, in init cyvcf2.cyvcf2
ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

Versions:

  • cyvcf2: 0.31.0
  • numpy: 2.0.0

I had to downgrade numpy manually to numpy 1.26.4 to get cyvcf2 working again.

@aryarm
Copy link
Contributor

aryarm commented Jun 17, 2024

I'm also running into this issue.

Based on the numpy docs, I think the pyproject.toml file needs to be updated.

[build-system]
requires = [
    "setuptools",
    "wheel",
    "cython>=0.23.3",
-    'oldest-supported-numpy; os_name != "nt"',
+    'oldest-supported-numpy; os_name != "nt"' and python_version < "3.9"',
-    'numpy; os_name == "nt"'
+    'numpy>=2.0.0; os_name == "nt" and python_version >= "3.9"',
]

brentp added a commit that referenced this issue Jun 17, 2024
brentp added a commit that referenced this issue Jun 17, 2024
@brentp
Copy link
Owner

brentp commented Jun 17, 2024

@aryarm , I pushed your suggestions, but the build is still failing. Any suggestions?

@brentp brentp closed this as completed Jun 17, 2024
@brentp brentp reopened this Jun 17, 2024
@aryarm
Copy link
Contributor

aryarm commented Jun 17, 2024

@brentp, I think I made a mistake! I didn't realize os_name == "nt" applies only to windows

In that case, maybe try something more like this?

[build-system]
requires = [
    "setuptools",
    "wheel",
    "cython>=0.23.3",
-    'oldest-supported-numpy; os_name != "nt"',
+    'oldest-supported-numpy; os_name != "nt"' and python_version < "3.9"',
-    'numpy; os_name == "nt"'
+    'numpy; os_name == "nt" and python_version < "3.9"',
+    'numpy>=2.0.0; python_version >= "3.9"',
]

In other words:

  • if the python version is 3.9+, use numpy>=2.0.0
  • otherwise
    • if the os is windows, use any compatible numpy
    • or use oldest-supported-numpy if the os is linux

@aryarm
Copy link
Contributor

aryarm commented Jun 17, 2024

you might also need to make some changes to the source code to make it compatible with numpy 2.0

This doc has a list of the required changes
https://numpy.org/doc/stable/numpy_2_0_migration_guide.html#c-api-changes

Just from a cursory glance, that might explain why the sdist build with python 3.12 failed? It seems to be the only test running against numpy 2.0. The rest of the tests seem to be running with an older version of numpy (even though they may be using numpy 2.0 for compilation!) because we haven't also updated the requirements.txt file yet

image

brentp added a commit that referenced this issue Jun 17, 2024
@brentp
Copy link
Owner

brentp commented Jun 17, 2024

I pushed to a branch with the updated requirements.txt. Will continue work on it later this week. Pull requests appreciated. ;)

@aryarm
Copy link
Contributor

aryarm commented Jun 17, 2024

ok! thanks for the quick response on this and for trying my suggestions!

It might also be a good idea to add steps to the CI to test against both the newest version of numpy and the oldest version of numpy supported by cyvcf2, as recommended in the numpy docs

@aryarm
Copy link
Contributor

aryarm commented Jun 18, 2024

ok, upon further investigation, it seems that the failing sdist build with python 3.12 from earlier was a fluke. It happened because we tried to build cyvcf2 with numpy < 2.0 and then run it with a version of numpy > 2.0, which is explicitly discouraged in the docs

The CI seems to pass now in #308 on my fork, so I'm inclined to say that there won't need to be any changes to the source code to support numpy 2.0, after all

@brentp
Copy link
Owner

brentp commented Jun 18, 2024

excellent! thank you for figuring this out!

@brentp
Copy link
Owner

brentp commented Jun 18, 2024

I pushed v0.31.1 with changes from @aryarm . Should resolve this issue.

@aryarm
Copy link
Contributor

aryarm commented Jun 18, 2024

seems to be working now on my end! feel free to close this

thanks for the quick turnaround on this!

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

No branches or pull requests

3 participants