BUG: apply NEP 50 scalar rules in concatenate and choose, and make casting="no" match "equiv" - #32510
Closed
Rohan143-mp wants to merge 1 commit into
Closed
Rohan143-mp wants to merge 1 commit into
Rohan143-mp wants to merge 1 commit into
Conversation
…sting=no match equiv Closes numpy#32491
Author
|
Hi @ngoldbaum, I've implemented the fix for #32491 along with unit tests and docs. Whenever you have time, I'd appreciate your review and feedback. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #32491
Problem & Background
In ufuncs and
np.copyto, passingcasting="no"with a Python scalar whose target dtype differs from its default dtype (e.g.int64for Pythonint,float64for Pythonfloat) was previously accepted, whilecasting="equiv"raised aTypeError:Because "no" casting is strictly more restrictive than "equiv", "no" should never succeed where "equiv" fails.
Root Cause
In numpy/_core/src/multiarray/abstractdtypes.c, npy_update_operand_for_scalar specifically checked for casting == NPY_EQUIV_CASTING. When casting == NPY_NO_CASTING was passed, it fell through, recreated the temporary array with the target descriptor, and the subsequent cast-safety check compared identical dtypes.
Additionally:
np.concatenate(axis=None) and np.choose did not convert Python scalars using NEP 50 value bounds, causing out-of-bounds integers to silently wrap instead of raising OverflowError (e.g., np.concatenate((np.ones(2, "int8"), 300), axis=None) returned 44 for 300).
In abstractdtypes.c, npy_update_operand_if_pystr was only handling Python str, but the same scalar re-conversion logic was needed for all Python literals/scalars (NPY_ARRAY_WAS_PYTHON_LITERAL).
Changes Made
abstractdtypes.c:
Included convert_datatype.h for npy_casting_to_string.
Changed condition in npy_update_operand_for_scalar from casting == NPY_EQUIV_CASTING to casting <= NPY_EQUIV_CASTING, rejecting both "no" and "equiv" when the scalar's default dtype does not match the target. Formatted the exception message dynamically using npy_casting_to_string(casting).
Generalized npy_update_operand_if_pystr to npy_update_operand_if_pyscalar, handling NPY_ARRAY_WAS_PYTHON_LITERAL | NPY_ARRAY_WAS_PYTHON_STR and forwarding the operation's casting argument.
abstractdtypes.h:
Updated prototype for npy_update_operand_if_pyscalar.
convert_datatype.c:
Updated PyArray_ConvertToCommonType (used in np.choose) to call npy_update_operand_if_pyscalar(&mps[i], op, i, common_descr, NPY_SAFE_CASTING).
multiarraymodule.c:
Updated PyArray_ConcatenateFlattenedArrays (np.concatenate(..., axis=None)) to call npy_update_operand_if_pyscalar(&arrays[iarrays], op, iarrays, PyArray_DESCR(ret), casting).
Documentation & Release Notes
Added Towncrier release notes:
doc/release/upcoming_changes/32497.compatibility.rst
doc/release/upcoming_changes/32497.improvement.rst
Updated doc/source/glossary.rst casting definition with reference to scalar rules.
Added a dedicated section Cast safety of Python scalars in doc/source/reference/arrays.promotion.rst documenting kind-based promotion, precision loss, out-of-bounds OverflowError, and "no" / "equiv" restrictions.
Unit Tests
test_api.py: Added assertions in test_copyto_cast_safety verifying that casting="no" raises TypeError when target dtypes differ.
test_multiarray.py: Fixed in-bounds scalar values in test_output_dtype and added test_pyscalar_out_of_bounds for np.choose.
test_shape_base.py: Added test_pyscalar_out_of_bounds, test_pyscalar_casting_matches_copyto, and test_pyscalar_casting_matches_ufunc.
test_stringdtype.py: Removed obsolete assertions in test_pystr_scalar_concatenate_preserves_nulls that expected "no" and "equiv" to succeed for Python strings.
test_ufunc.py: Added casting="no" tests for scalar operands in test_cast_safety_scalar and test_resolve_dtypes_basic.
Result
casting="no" and casting="equiv" now behave consistently: if converting a scalar to a non-default dtype fails under "equiv", it also fails under "no".
Out-of-bounds scalar values now consistently raise OverflowError rather than silently overflowing or wrapping:
Full Parity Across Operations:
np.concatenate(axis=None), np.choose, np.copyto, and ufuncs now share the exact same scalar casting and conversion behavior.