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

MAINT: redefine PyArray_MAX/PyArray_MIN because they moved in NumPy #18967

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

tupui
Copy link
Member

@tupui tupui commented Jul 26, 2023

Closes #18966

@tupui tupui added upstream bug Items related to bugs in upstream projects C/C++ Items related to the internal C/C++ code base labels Jul 26, 2023
@rgommers rgommers self-requested a review July 26, 2023 10:45
@tupui
Copy link
Member Author

tupui commented Jul 26, 2023

@rgommers @ev-br this seems to fix the issue 😃

@rgommers rgommers added this to the 1.12.0 milestone Jul 27, 2023
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to make CI happy, but redefining PyArrow_MAX without #ifndef guards may potentially run into more trouble later. Given that this macro nor the code that uses it have anything to do with NumPy arrays, I suggest renaming the macro to _MAX_OF_TWO_VALUES or some such name.

(fun with obscure C, in C++ we could just have std::max).

@rgommers
Copy link
Member

The PyArray_MIN one is different:

  thetype = PyArray_ObjectType(image, NPY_FLOAT);
  thetype = PyArray_MIN(thetype, NPY_DOUBLE);

that still deals with integer type codes, but I guess we can clean that up since PyArray_ObjectType is superceded since forever: https://numpy.org/devdocs/reference/c-api/array.html#c.PyArray_ObjectType.

Okay, maybe let's get this in to unbreak CI, and deal with the rest in a follow-up.

@rgommers rgommers merged commit 6c00b00 into scipy:main Jul 27, 2023
25 checks passed
@rgommers rgommers changed the title MAINT: refactor PyArray usages MAINT: redefine PyArray_MAX/PyArray_MIN because they moved in NumPy Jul 27, 2023
@tupui tupui deleted the pyarray_fix branch July 27, 2023 15:46
scottshambaugh pushed a commit to scottshambaugh/scipy that referenced this pull request Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C/C++ Items related to the internal C/C++ code base upstream bug Items related to bugs in upstream projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: PyArray_MAX no longer available in numpy/arrayobject.h
2 participants