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

Fix clipping float dtypes when either min or max are Python ints #276

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

magnusdk
Copy link
Contributor

This PR makes clip no longer crash when x has a floating point dtype and min and/or max are Python ints.

import array_api_compat.numpy as xp
xp.clip(xp.asarray(0.0), 0, 1)  # <- this used to crash

The Array API for clip states that behavior is unspecified if xand either min or max have different data type kinds, but I think the change in this PR makes it behave more like what one might expect. This change also makes the code more similar to NumPy's handling of Python ints in clip.

This is my first PR for this repo, hopefully I got it right :) Let me know if I should add anything else.
The work you do with array-api is great, thanks a lot!

@ev-br ev-br merged commit e4168e7 into data-apis:main Mar 19, 2025
40 checks passed
@ev-br
Copy link
Member

ev-br commented Mar 19, 2025

I checked locally that indeed, the behavior of numpy, pytorch, jax.numpy and dask.array is to allow xp.clip(xp.asarray(0.0), 0, 1) and return a floating-point 0.0.
The wrapped namespaces all currently reject this invocation.

The behavior is indeed unspecified in the standard, and is thus implementation-defined. array-api-compat by design should not disallow implementation-defined behaviors.

The itself change looks safe.

Therefore, LGTM.

Let's merge this patch, thank you @magnusdk!

@ev-br ev-br added this to the 1.12 milestone Mar 19, 2025
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

Successfully merging this pull request may close these issues.

2 participants