Skip to content

Conversation

@Enderdead
Copy link
Contributor

@Enderdead Enderdead commented Oct 2, 2025

This merge request introduces the setdiff1d delegate function (#100).

Nothing special to mention here — it was NOT a straightforward contribution.

@Enderdead Enderdead marked this pull request as draft October 2, 2025 15:16
@lucascolley lucascolley self-requested a review October 2, 2025 16:15
@lucascolley lucascolley added this to the 0.9.1 milestone Oct 2, 2025
@lucascolley lucascolley added enhancement New feature or request delegation labels Oct 2, 2025
@Enderdead Enderdead changed the title ENH: setdiff1d delegate function WIP ENH: setdiff1d delegate function Oct 2, 2025
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

see also #124

@Enderdead
Copy link
Contributor Author

Enderdead commented Oct 7, 2025

Two issues have been identified during the development of the JAX part (see jax-ml/jax#32335 and jax-ml/jax#32402 )

I will finish my contribution once all the issues have been reported to the JAX library.

@Enderdead
Copy link
Contributor Author

According to my understanding of the work discussed in #124, the JAX JIT version has been postponed indefinitely, but the Dask version is still desired. Since the function setdiff1d is not available in the Dask API (see Dask NumPy compatibility), the implementation proposed by @crusaderky seems worth pushing forward.

Because this implementation differs from a classic delegate function (as it introduces a new approach), where should I place the dedicated code for Dask? Alternatively, do we want to replace the current version, which uses the _helpers.in1d method, with @crusaderky’s implementation?

@lucascolley
Copy link
Member

I'm not sure about how to proceed for Dask and JAX, but I would be happy to review a PR which just adds delegation for the other libraries.

@Enderdead Enderdead force-pushed the setdiff1d-delegate-function branch from 6887ffb to 62f3d6c Compare October 20, 2025 08:31
@Enderdead Enderdead force-pushed the setdiff1d-delegate-function branch 2 times, most recently from 320a1d2 to 2f1eadb Compare October 20, 2025 08:40
@Enderdead Enderdead changed the title WIP ENH: setdiff1d delegate function ENH: setdiff1d delegate function Oct 20, 2025
@Enderdead Enderdead marked this pull request as ready for review October 20, 2025 08:41
@lucascolley lucascolley changed the title ENH: setdiff1d delegate function ENH: setdiff1d: add delegation Oct 21, 2025
@lucascolley lucascolley self-requested a review October 21, 2025 10:13
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Enderdead !

Copy link
Contributor

@crusaderky crusaderky left a comment

Choose a reason for hiding this comment

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

Just a nit on the tests.

I checked that the documentation builds correctly.

@Enderdead
Copy link
Contributor Author

@crusaderky Corrections made to the tests.

@lucascolley
Copy link
Member

Did you mean to include a change to atleast_nd too?

@Enderdead
Copy link
Contributor Author

@lucascolley I swapped the function order in the file to follow alphabetical order and added the missing item to the __all__ list. Is that a problem? I can split this into a second commit for clarity.

@lucascolley
Copy link
Member

all good, thanks again @Enderdead ! And for the review @crusaderky

@lucascolley lucascolley merged commit 24facbe into data-apis:main Oct 28, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants