Skip to content

Conversation

@Enderdead
Copy link
Contributor

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

Unlike other delegate functions, atleast_nd does not exist for all major backends. Instead, the functions atleast_1d, atleast_2d, and atleast_3d are available.

In this implementation, I use getattr to delegate when ndim is between 1 and 3. I’ve also added tests with ndim=3 to cover the full range.

Open to feedback!

@lucascolley lucascolley self-requested a review October 2, 2025 16:16
@lucascolley lucascolley changed the title ENH: atleast_nd delegation ENH: atleast_nd delegation Oct 2, 2025
@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
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.

looks great, I agree that this delegation strategy makes sense, thanks @Enderdead !

@lucascolley lucascolley merged commit 708bce7 into data-apis:main Oct 3, 2025
11 checks passed
@mdhaber
Copy link
Contributor

mdhaber commented Nov 5, 2025

@lucascolley @Enderdead git bisect finds that something about this commit (708bce7) causes tests of scipy.spatial.transform.RigidTransform to fail. See e.g. first test run of scipy/scipy#23929.

@lucascolley
Copy link
Member

Thanks for the bisection, I'll try to investigate today

@lucascolley
Copy link
Member

Some of RigidTransform is relying on atleast_nd adding dimensions to the front of the shape, but np.atleast_3d adds dimensions to the back:

(Pdb) x.shape
(3, 3)
(Pdb) np.atleast_3d(x).shape
(3, 3, 1)
(Pdb) x = np.arange(4)
(Pdb) x.shape
(4,)
(Pdb) np.atleast_2d(x).shape
(1, 4)

@lucascolley
Copy link
Member

@Enderdead would you be interested in opening a PR to document that we add dimensions to the front of the shape, and removing the delegation to np.atleast_3d?

@lucascolley
Copy link
Member

Apparently according to the docs, np.atleast_3d applied to a 1-D array sandwiches the existing axis between new axes 🤔 https://numpy.org/doc/2.3/reference/generated/numpy.atleast_3d.html#numpy-atleast-3d

@amacati
Copy link

amacati commented Nov 5, 2025

That's a weird decision by numpy, didn't know about that. I think the version assumed in RigidTransform still makes a lot of sense.

@Enderdead
Copy link
Contributor Author

@lucascolley No problem, I’ll take care of fixing this issue. I haven’t looked into it deeply yet, but it’s concerning that the proposed unit test didn’t catch this behavior mismatch. I’ll try to push a fix by the end of the week (check unit test + remove delegation).

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.

4 participants