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: fix ValueError in PyArray_Std on win_amd64 #19011

Merged
merged 1 commit into from
May 18, 2021
Merged

BUG: fix ValueError in PyArray_Std on win_amd64 #19011

merged 1 commit into from
May 18, 2021

Conversation

cgohlke
Copy link
Contributor

@cgohlke cgohlke commented May 14, 2021

Fixes the following type of ValueError on 64-bit Python for Windows, which is due to casting of 64-bit ssize_t to 32-bit long :

# test.pyx
import sys
import numpy
from numpy cimport PyArray_Std, NPY_FLOAT, ndarray, import_array

import_array()
print(sys.version)

cdef ndarray a = numpy.zeros((2**32 + 1023, 1), dtype='uint8')
PyArray_Std(a, -1, NPY_FLOAT, None, 0)
# setup.py
from setuptools import setup, Extension
import numpy

setup(
    name='test',
    ext_modules=[
        Extension('test', ['test.pyx'], include_dirs=[numpy.get_include()])
    ],
)
> python setup.py build_ext --inplace
> python -c"import test"
3.9.5 (tags/v3.9.5:0a7dcbd, May  3 2021, 17:27:52) [MSC v.1928 64 bit (AMD64)]
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "test.pyx", line 10, in init test
    PyArray_Std(a, -1, NPY_FLOAT, None, 0)
ValueError: cannot reshape array of size 4294968319 into shape (1023,1)

@cgohlke
Copy link
Contributor Author

cgohlke commented May 14, 2021

Other suspicious uses of PyLong_FromLong:

PyTuple_SET_ITEM(ret, idim,
PyLong_FromLong(shape[idim]));

PyTuple_SET_ITEM(ret, idim,
PyLong_FromLong(multi_index[idim]));

@seberg
Copy link
Member

seberg commented May 14, 2021

Well, you are right... All of those should use the Ssize_t version... I guess we could just put this in. These C-version for std, etc. are a bit awkward, since we don't actually use them from Python, meaning that there are very few to no users (and they might diverge).

Should we try to add tests and/or deprecate these functions entirely? It also seems silly that the code (still) uses PyArray_Reshape when PyArray_NewShape is a much better fit (this builds a tuple, just to deconstruct it again immediately!).

@mattip
Copy link
Member

mattip commented May 14, 2021

Should we try to add tests and/or deprecate these functions entirely? It also seems silly that the code (still) uses PyArray_Reshape when PyArray_NewShape is a much better fit (this builds a tuple, just to deconstruct it again immediately!).

Probably a different PR, maybe one for a beginner sprint?

Other suspicious uses of PyLong_FromLong ...

Could those be added to this PR since they are bug fixes?

@mattip mattip added the 09 - Backport-Candidate PRs tagged should be backported label May 14, 2021
@mattip
Copy link
Member

mattip commented May 14, 2021

I wonder why we didn't get compiler warnings for the downcasts

@charris charris merged commit df82106 into numpy:main May 18, 2021
@charris
Copy link
Member

charris commented May 18, 2021

Thanks Christoph,

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

Successfully merging this pull request may close these issues.

None yet

4 participants