-
Notifications
You must be signed in to change notification settings - Fork 119
Fix import error on aarch64: define 'long' using ctypes.c_long #146
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
base: master
Are you sure you want to change the base?
Fix import error on aarch64: define 'long' using ctypes.c_long #146
Conversation
### Fix issue with PyOpenGL-accelerate on MacOS x86_64 ### Linked issues n/a ### Summarize your change. This PR adds a temporary changes to build PyOpenGL-accelerate from source on MacOS x86_64. The source is patched with the changes from a pending [PR](mcfletch/pyopengl#146) in the PyOpenGL repository. ### Describe the reason for the change. Cython was updated to version 3.1.0+ on May 8, causing compatibility issues with PyOpenGL-accelerate on macOS x86_64. Until a pending [PR](mcfletch/pyopengl#146) is merged in the official PyOpenGL repository, PyOpenGL-accelerate will be built from source on macOS x86_64. ### Describe what you have tested and on which operating system. ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Cédrik Fuoco <[email protected]>
…undation#777) ### Fix issue with PyOpenGL-accelerate on MacOS x86_64 ### Linked issues n/a ### Summarize your change. This PR adds a temporary changes to build PyOpenGL-accelerate from source on MacOS x86_64. The source is patched with the changes from a pending [PR](mcfletch/pyopengl#146) in the PyOpenGL repository. ### Describe the reason for the change. Cython was updated to version 3.1.0+ on May 8, causing compatibility issues with PyOpenGL-accelerate on macOS x86_64. Until a pending [PR](mcfletch/pyopengl#146) is merged in the official PyOpenGL repository, PyOpenGL-accelerate will be built from source on macOS x86_64. ### Describe what you have tested and on which operating system. ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Cédrik Fuoco <[email protected]> Signed-off-by: [email protected] <[email protected]>
…undation#777) ### Fix issue with PyOpenGL-accelerate on MacOS x86_64 ### Linked issues n/a ### Summarize your change. This PR adds a temporary changes to build PyOpenGL-accelerate from source on MacOS x86_64. The source is patched with the changes from a pending [PR](mcfletch/pyopengl#146) in the PyOpenGL repository. ### Describe the reason for the change. Cython was updated to version 3.1.0+ on May 8, causing compatibility issues with PyOpenGL-accelerate on macOS x86_64. Until a pending [PR](mcfletch/pyopengl#146) is merged in the official PyOpenGL repository, PyOpenGL-accelerate will be built from source on macOS x86_64. ### Describe what you have tested and on which operating system. ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Cédrik Fuoco <[email protected]> Signed-off-by: [email protected] <[email protected]>
I think this is only an issue with Cython 3 -- but we should be using Cython 3 by now! Does this seem to work with Cython3 across the board? NOTE: I'm working on a conda-forge recipe -- so I may apply this as a patch to build .... (though pinning to Cython<3 may be the more expedient route...) |
Got it all working on conda-forge -- but I had to add this patch -- so it would be nice to get this merged. I do wonder how the wheels got built ???? |
Yes, this fix should work across the board with Cython 3. Using |
Indeed -- and it is working for me -- with this patch and Cython 3 -- and with Cython <3, I got segfaults. I think -- numpy 2 changed a bit how it handles the C long types, which might explain the segfaults. I've had other issues with that). I curse C for defining "short" and "long" way back when, rather than the modern int32 and int64, etc ... oh well. Anyway, no need to understand all the details -- what I think we need isthis patch and also to pin Cython to >3 Maybe add that pin to this PR? Also: I see in the MANIFEST.in file that the generated c files are still being included in the source distro -- that probably should change. |
As you requested, I've added the version pin to require |
Thanks! now we just need a merge ... |
I've been looking forward to this for a month. 😄 |
@mcfletch: This is actually pretty bad -- pip install of accelerate on OS-X Intel fails for this reason. There is no wheel for OS-X Intel -- so ti tries to build from source. Also -- I'm curious how the wheels that are there were successfully built, and if they actually work (I could build with Cython < 3, but got segfaults when trying to run the tests). BTW -- I'm sure there's a couple of us that would be glad to help with maintenance of you don't have the time. |
NOTE: I've got a build for conda-forge working: https://anaconda.org/conda-forge/pyopengl-accelerate But I needed to apply this patch to get it to build -- see: https://github.com/conda-forge/pyopengl-accelerate-feedstock |
While this might fix the compile issue, I'm suspicious that it might not be a correct fix. I'm seeing these test failures after applying the patch.
|
I see you're using python 3.14 -- maybe somethting has changed there. What's the version of everything else?
|
You should try using |
Well, it should match what the OpenGL headers have. On some platforms, int and long are the same, and others not -- one of those really annoying things about C! Also, rather than the ctypes long or int, we should use cython.long or cython.int. https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#types |
One more note: This patch worked, and passed the tests, on all the platforms supported by conda-forge. https://github.com/conda-forge/pyopengl-accelerate-feedstock Including windows, where int and long are the same, and Linux and Mac where they are not. So I think long is correct. |
But the code in question is Python code, not C code. So I think @totaam is correct that EDIT: answered my own question. It looks like pyopengl-accelerate-feedstock is running part of the test suite, but not all of it.
|
@ChrisBarker-NOAA I would have to look at the generated C code to be sure, but doesn't using |
I don't have time to dig in , but in Cython (particularly used in this way) you are setting the type to match the C type, so it doesn't have to do Python<->C conversions -- so you really want the C type that matches the C type used in the OpenGL code you are calling. Key is that this DOES work with the whole set of platforms the conda-forge builds for: It runs the opengl-accelerate tests -- it doesn't run the PyOpenGL tests, as those require OpenGL on the test runners. maybe those could be run on Windows and/or the Mac -- though I think that I had trouble getting them to run locally, so give up trying. Actually -- what we should have is tests on the gitHub CI -- which I started, but there's the whole headless problem: But back to this issue: IIUC, the "new" problem was on python 3.14 and I don't know what numpy versiuon or platform -- This error may have nothing to do with this patch. The error is "TypeError: 'int' object is not subscriptable" C int, C long, python int -- none of them are subscriptable. I think the problem is here:
which means that buffers isn't typed, so it's whatever:
is returning ... this is all quite confusing, as what I see above (line 39)
so back to the code at hand:
that may well have been a conversion to a Python2 long (required because a Python2 int couldn't hold the full range of a C unsigned int) -- and then Cython is auto-converting it to a C unsigned int. If that's the case, then you'd want to use As suggested here -- as a Python3 int is a Python2 long. I wonder what type buffers is ??? Why isn't that known at this point??? Anyway -- lots of testing to be done ... Antoher hint:
so that must have been a Python2 long |
I really don't have the time for this, but since I was thinking about it.... I tested just using new PR here: @vsevolod-misiul: -- I'm pretty sure this is the right way to go -- so if you could test my PR, that would be great -- and then you can close this one. |
Just confirmed on my end as well, switching to I submitted a PR as well with just the |
Yet another note: Turns out that the tests are not run on the conda packages on Linux:
that's because they failed, probably due to missing OPenGL on the headless CI servers -- so maybe it's been broken on Linux all along. |
Summary
On Linux ARM64 (aarch64) platforms, installing PyOpenGL_accelerate fails with the following error:
This happens because the built-in long type is not defined in Python 3 on ARM64 platforms, unlike some legacy Python 2 or x86-based assumptions. This breaks Cython compilation when trying to cast buffer identifiers.
This patch imports
long
fromctypes.c_long
, which resolves the issue on these platforms.Changes
from ctypes import c_long as long
to handle missinglong
type on aarch64.Platform tested