ENH: Added atleast_nd #7804

madphysicist · 2016-07-06T03:58:47Z

Introduced generalized atleast_nd function. Similar to atleast_1d, atleast_2d, atleast_3d. Also added for masked arrays. Tests included.

This PR also fixes an issue with masked arrays that would crash the atleast_* functions if multiple scalars were input, e.g. np.ma.atleast_1d(1.0, 2.0, 3.0) would raise an error.

mhvk · 2016-07-06T14:34:37Z

numpy/core/shape_base.py

+    """
+    res = []
+    for ary in arys:
+        ary = asanyarray(ary)


As noted on the mailing list, if we go for this at all, you can just write

ary = array(ary, copy=False, subok=True, ndim=n)

Not if I add an option to place the dimensions at the end. I may still work this in without the ndim argument for that case though.

I did not use your suggestion literally because zero-dim arrays need special handling, but I did something quite similar, just without the ndmin parameter (it's not ndim for array).

madphysicist · 2016-07-14T14:57:13Z

So yeah...

madphysicist · 2016-07-26T13:37:59Z

bump

juliantaylor · 2016-08-17T14:05:04Z

can you implement the lower dimension variants in terms of this one?

madphysicist · 2016-08-17T15:34:53Z

Not atleast_3d until I introduce a more complex spec for adding dimensions. atleast_3d adds dimensions on both sides of a 1D array. The other two are fine.

juliantaylor · 2016-08-17T15:42:56Z

unless this function is significantly slower I would suggest to implement 1d and 2d in terms of this one for the extra test coverage and less code.

madphysicist · 2016-08-17T17:17:35Z

Done. Reduced atleast_[12]d to three lines of code. Could have probably been one, but the input parameters are weird.

homu · 2016-08-27T09:50:22Z

☔ The latest upstream changes (presumably #7823) made this pull request unmergeable. Please resolve the merge conflicts.

shoyer · 2016-08-28T20:20:41Z

numpy/core/shape_base.py

+    -------
+    res : ndarray
+        An array with ``a.ndim >= n``. Copies are avoided where
+        possible, and a view with `n` or more dimensions is returned.


Is a copy ever needed?

I do not know enough about the internals to know if res = array(ary, copy=False, subok=True) will ever return a copy under normal circumstances. My guess is no, but I wanted to do some mild CYA in the docs, just in case.

It will only make a copy for non numpy arrays (e.g., lists).
On Mon, Aug 29, 2016 at 8:12 AM Joseph Fox-Rabinovitz <
notifications@github.com> wrote:

In numpy/core/shape_base.py
#7804 (comment):

ary : array_like

The input array. Non-array inputs are converted to arrays.

Arrays that already have `n` or more dimensions are preserved.

n : scalar

The minimum number of dimensions required for each of the input

arrays.

pos : {'start', 'end'}, optional

Where to add new dimensions. All new dimensions are added on the

same side of the shape. Default is 'start'.

Returns

res : ndarray

An array with `a.ndim >= n`. Copies are avoided where

possible, and a view with `n` or more dimensions is returned.

I do not know enough about the internals to know if res = array(ary,
copy=False, subok=True) will ever return a copy under normal
circumstances. My guess is no, but I wanted to do some mild CYA in the
docs, just in case.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/numpy/numpy/pull/7804/files/5b443132ec6c30aef974c73b30b510e6b60e8755#r76622781,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABKS1p0XcxxYEGc_49tnk41VdMQUHXgoks5qkvSFgaJpZM4JFvBv
.

I have updated the docstring to reflect that now.

homu · 2016-10-20T22:15:38Z

☔ The latest upstream changes (presumably #7922) made this pull request unmergeable. Please resolve the merge conflicts.

mattharrigan · 2016-10-26T17:59:49Z

see issue 8214

mattharrigan · 2016-10-26T18:03:39Z

numpy/core/shape_base.py

+        Arrays that already have `n` or more dimensions are preserved.
+    n : scalar
+        The minimum number of dimensions required for each of the input
+        arrays.


there's only ever 1 array, for each of the input arrays doesn't seem to make sense

Good call. Fixed now.

shoyer · 2016-10-26T18:52:42Z

This looks pretty solid to me. Did we ever discuss it on the mailing list?

Two suggestions:

Note that the default pos='start' creates arrays that are consistent with NumPy's broadcasting rules.
atleast_3d should have a loud note that its implementation is inconsistent with the others, for legacy reasons.

madphysicist · 2016-10-26T19:19:16Z

Mailing list thread start: https://mail.scipy.org/pipermail/numpy-discussion/2016-July/075759.html. There was no solid consensus to merge, but no strong objections either if I remember correctly.

madphysicist · 2016-10-26T19:28:38Z

I think that suggestion 2 is adequately covered in the notes section already. I added another entry for suggestion 1.

shoyer · 2018-04-17T15:52:21Z

A couple of comments on the API:

Let's call the parameter for specifying the number of dimension ndim. atleast_nd(array, ndim=3) is self-explanatory in a way that atleast_nd(array, n=3) is not. Many people will skip the keyword argument, but if you're going to use it anyways it's better to have something descriptive. NumPy has too many functions already with non-descriptive argument names.
I am -1 on an inplace argument, for numerous reasons:
- As @eric-wieser notes, modifying shape in place is an antipattern, which should be discouraged as much as possible.
- None of the other array manipulation functions in NumPy have an inplace argument.
- It's not clear if what the return value is now. Is it None like most inplace functions or the original array?
- There is nothing to be gained from an efficiency or code expressiveness perspective from using atleast_nd(array, 3, inplace=True) rather than array = atleast_nd(array, 3)

madphysicist · 2018-04-17T17:01:55Z

@shoyer. I've squashed in all the changes you requested. inplace really does not make sense when you put it that way.

eric-wieser · 2018-04-17T17:16:15Z

numpy/core/shape_base.py

+        The minimum number of dimensions required.
+    pos : int, optional
+        The index to insert the new dimensions. May range from
+        ``-ary.ndim`` to ``+ary.ndim``. Non-negative indices indicate


Not true - the lower bound is -ary.ndim - 1

Thanks for the catch. I had a feeling at the back of my head that the indices should not be the same, but was in too much of a hurry to place it. Fixed now.

eric-wieser · 2018-04-17T17:19:17Z

@Erotemic

In these cases there is only one new dimension or all dimensions are inserted (so ordering doesn't matter)

I agree

I would absolutely expect the new axis to be grouped together.

Can you give a concrete example of where you'd expect this? As you note, for the examples you've given so far, it doesn't matter.

Given that we've come up with no convincing use cases for intermediate values, perhaps we should just allow only pos=0 and pos=-1?

madphysicist · 2018-04-17T17:57:17Z

@eric-wieser The only use-case I can think of is something I did a while back where I needed to do a dot product over the last two dimensions in a 2D image that contained essentially small 2D matrices for each pixel, which would eventually be converted to color values (so a 4D array). After all the axis rolling, it turned out that the best way to broadcast to the particular RGB format I was interested in was to insert two unit axes at index 2. I honestly can't remember the exact details, and I'm sure there is a better way to do it, but I think it's better to split the existing dimensions rather than the new ones. I would also like to leave the ability to insert into an arbitrary index, not just 0 and -1.

madphysicist · 2018-04-19T20:00:48Z

I forgot to remove the inplace parameter in the function signature. Fixed now. I believe that fixes the last remaining issue, until the next one.

madphysicist · 2018-04-24T19:14:55Z

Rebased. Is everyone more or less happy with the state of this PR?

Erotemic · 2018-04-27T10:42:15Z

@eric-wieser

Can you give a concrete example of where you'd expect this? As you note, for the examples you've given so far, it doesn't matter.

Say I have a 1D array with shape (N,), and I want to interpret it as an image so I can pass it to my image processing tools that all expect 3D arrays where the first two dimensions are spatial and the last are channels. To ensure that inputs are in correct formats I would use atleast_nd with n=3 and pos=-1. In this case I would expect the new shape to be (N, 1, 1). A similar case might arise when working with 4D tensor libraries like pytorch. I don't think its an entirely common case, but I think it would exist and in that rare case I would expect new axis to be grouped.

I think the key idea that illustrates my expectation is what happens when there are 3 new axes? Will there be two at the front and one at the end or the other way. It seems ambiguous to allow for any other case than grouped axes.

Given that we've come up with no convincing use cases for intermediate values, perhaps we should just allow only pos=0 and pos=-1?

This is my preference, but I'm not attached to it. I just think its conceptually easier to understand document and use. It prevents users from shooting themselves in the foot.

madphysicist · 2018-04-27T21:40:40Z

@Erotemic While I agree with you that grouping the new axes rather than the existing ones is by far the more sensible approach, I prefer to allow positions other than 0 and -1. For the work that I do, the color planes are stored separately, so it is often easier to have the channel number as the first rather than the last dimension. In that case it would be very helpful to insert the unit dimension into index -3.

madphysicist · 2018-07-10T16:07:46Z

Bump?

Function only accepts one array and an indexed placement spec. Masked arrays fully supported. atleast_1d and atleast_2d re-implemented in terms of new func. Tests included.

charris · 2018-09-30T18:36:05Z

@eric-wieser Want to revisit this? You seemed the most critical.

@madphysicist Was this discussed on the list at some point?

madphysicist · 2018-10-01T11:29:27Z

@charris. At some length, but a very long time ago: https://mail.python.org/pipermail/numpy-discussion/2016-July/075722.html

ahaldane · 2018-10-02T21:42:35Z

I only skimmed the discussions, but:

As syntactic sugar, I think it's a good addition. I was just looking at code that would use this: The block PRs happen to define their own _atleast_nd for clarity, and in multiarraymodule.c there is a C function _prepend_ones which essentially does this, used by the C code for np.concatenate, and for the ndim kwd of np.array.

Suggestion for this PR: Optimize for the most common case where pos == 0, by having a clause if pos == 0: return np.array(a, ndmin=ndim, copy=False, subok=True) at the start. That would make nice use of the _prepend_ones code path.

ahaldane · 2018-10-02T21:59:00Z

numpy/core/shape_base.py

+    [[[[1, 2, 3]]]]
+    """
+    ary = array(ary, copy=False, subok=True)
+    if ary.ndim:


why this if?

Why not move the pos = line into the if extra >0 block?

ahaldane · 2018-10-02T22:02:02Z

numpy/core/shape_base.py

+        pos = normalize_axis_index(pos, ary.ndim + 1)
+    extra = ndim - ary.ndim
+    if extra > 0:
+        ind = pos * (slice(None),) + extra * (None,) + (Ellipsis,)


This code reminds me it would be nice to have an axis argument to put: see issue #8765

charris · 2020-12-17T17:47:20Z

This addition looks desirable. @madphysicist I'm going to close this, but feel free to rebase and resubmit. That will bring it closer to the head of the list :)

mhvk reviewed Jul 6, 2016
View reviewed changes

madphysicist force-pushed the atleast_nd branch 3 times, most recently from d432669 to 40b299a Compare July 7, 2016 18:05

charris added 01 - Enhancement component: numpy._core labels Jul 8, 2016

madphysicist force-pushed the atleast_nd branch 2 times, most recently from 38ca1ce to 75a9e16 Compare July 11, 2016 14:29

madphysicist mentioned this pull request Jul 11, 2016

BUG: Fixed masked array behavior for scalar inputs to np.ma.atleast_*d #7823

Merged

madphysicist force-pushed the atleast_nd branch from 75a9e16 to 7831134 Compare August 17, 2016 17:16

madphysicist force-pushed the atleast_nd branch from 7831134 to 5b44313 Compare August 24, 2016 16:35

shoyer reviewed Aug 28, 2016
View reviewed changes

madphysicist force-pushed the atleast_nd branch 2 times, most recently from 8794a2e to 0f13b53 Compare August 29, 2016 15:42

madphysicist force-pushed the atleast_nd branch from 0f13b53 to 62b5724 Compare October 21, 2016 11:26

madphysicist mentioned this pull request Oct 24, 2016

ENH: add padding options to diff #8206

Merged

mattharrigan reviewed Oct 26, 2016

View reviewed changes

madphysicist force-pushed the atleast_nd branch from 47933b7 to b7ed12d Compare April 17, 2018 17:01

eric-wieser reviewed Apr 17, 2018

View reviewed changes

madphysicist force-pushed the atleast_nd branch from b7ed12d to d84ebd3 Compare April 17, 2018 17:52

madphysicist force-pushed the atleast_nd branch from d84ebd3 to d00e0b6 Compare April 19, 2018 19:59

madphysicist force-pushed the atleast_nd branch from d00e0b6 to 849afcf Compare April 24, 2018 19:14

madphysicist force-pushed the atleast_nd branch 3 times, most recently from c74febd to c8a0caa Compare July 9, 2018 20:53

ENH: Added atleast_nd

8720da5

Function only accepts one array and an indexed placement spec. Masked arrays fully supported. atleast_1d and atleast_2d re-implemented in terms of new func. Tests included.

madphysicist force-pushed the atleast_nd branch from c8a0caa to 8720da5 Compare September 25, 2018 17:23

ahaldane reviewed Oct 2, 2018

View reviewed changes

scottstanie mentioned this pull request Nov 5, 2018

atleast_2d axis argument option #12336

Open

charris closed this Dec 17, 2020

madphysicist mentioned this pull request Feb 10, 2021

ENH: Added atleast_nd to numpy and ma #18386

Open

madphysicist deleted the atleast_nd branch February 10, 2021 10:32

db091756 mentioned this pull request Jul 10, 2024

Implement custom atleast_2d shape function gchq/coreax#697

Merged

9 tasks

ENH: Added atleast_nd #7804

ENH: Added atleast_nd #7804

Conversation

madphysicist commented Jul 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madphysicist commented Jul 14, 2016

madphysicist commented Jul 26, 2016

juliantaylor commented Aug 17, 2016

madphysicist commented Aug 17, 2016

juliantaylor commented Aug 17, 2016

madphysicist commented Aug 17, 2016

homu commented Aug 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

homu commented Oct 20, 2016

mattharrigan commented Oct 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer commented Oct 26, 2016

madphysicist commented Oct 26, 2016

madphysicist commented Oct 26, 2016

shoyer commented Apr 17, 2018

madphysicist commented Apr 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser commented Apr 17, 2018 • edited Loading

madphysicist commented Apr 17, 2018

madphysicist commented Apr 19, 2018

madphysicist commented Apr 24, 2018

Erotemic commented Apr 27, 2018

madphysicist commented Apr 27, 2018

madphysicist commented Jul 10, 2018

charris commented Sep 30, 2018

madphysicist commented Oct 1, 2018

ahaldane commented Oct 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Dec 17, 2020

madphysicist commented Jul 6, 2016 •

edited

Loading

eric-wieser commented Apr 17, 2018 •

edited

Loading