-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Added atleast_nd to numpy and ma #18386
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
base: main
Are you sure you want to change the base?
Conversation
446be55
to
deee282
Compare
d30b529
to
9526541
Compare
I am pretty neutral on this addition (I honestly don't understand where its useful, but it also doesn't seem like a big feature creep – i.e. this is only useful when you don't know the input). If the old mailing list discussion was super positive, I might have overlooked it. But I think this has to hit the mailing list again with the concrete current proposal. I guess the old point is. If I just want to add dimensions on the left (usually not necessary), there are many options like |
ccfb39d
to
0a0729a
Compare
@seberg I am actually in a similar boat. There are a couple of cases where this would seem like a nice-to-have at best. I thought my original PR was dead and was prepared to leave it that way. The original feedback was pretty ambivalent, although I did have a few people ask me about it later. This is here pretty much because @charris asked me to revive it. |
a49ea85
to
aaabed2
Compare
@BvB93 It's pretty clear that the remaining errors are something I did in the typing tests, but I can't seem to figure it out. Could you take a look when you have the chance? I'm especially confused about the error saying that numpy does not have an attribute |
aaabed2
to
389e412
Compare
I've squashed all the commits now that the last bug is fixed. At this point, code is good to go unless someone has something to say on the new mailing list post. |
389e412
to
2474c47
Compare
Looks like the Azure failtures are not related to anything I did. |
|
||
|
||
@array_function_dispatch(_atleast_nd_dispatcher) | ||
def atleast_nd(ary, ndim, pos=0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In numpy.array you can pre-pend ones to the array-shape by giving a 'ndmin' parameter.
Therefore, I think the variable name 'ndim' should be renamed to 'ndmin' to make it consistent with numpy.array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the atleast_nd should allow multiple inputs in the same way as atleast_1d, atleast_2d and atleast_3d do. This will make it easier to use since then the call syntax and output will be the same as the atleast_XXXd functions. Any written code with multiple uses of the function will be shorter like this:
x1, x2, x3 = numpy.atleast_nd(x1,x2,x3, ndmin=3)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pos argument is the axis index to insert the new dimensions. What about renaming 'pos' to 'axis'? That will make it more consistent with the normalize_axis_index function.
I hope this will be merged. Otherwise, could anyone provide a simple way to do what this functionality provides? I.e. if my input is a 2d array - leave it as it is, otherwise if it is a 1d array, append one dimension to the end. Neither of above suggestions seem to do the trick. |
Hi, what's the status of this? Glad to help if anything is stoping it from going forward.
@seberg I think that's the whole point of For the use-case, I find the 2D shape of (event, dimensions) very common in many applications and therefore the desire to have one dimensional arrays made to 2D with shape (event, 1) Looking forward to have this in! |
I have been using this for a fair while now. Extremely useful. I don't even use the other ones
def atleast_nd(ary, ndim, pos=0):
ary = array(ary, copy=False, subok=True)
if ary.ndim > ndim:
pos = normalize_axis_index(pos, ary.ndim + 1)
shape = ary.shape
new_shape = shape[:pos] + (1,) * extra + shape[pos:]
return reshape(a, new_shape)
return ary
def atleast_nd(ary, ndim, pos=0, exact=False):
ary = array(ary, copy=False, subok=True)
extra = ary.ndim > ndim
if extra > 0:
pos = normalize_axis_index(pos, ary.ndim + 1)
shape = ary.shape
new_shape = shape[:pos] + (1,) * extra + shape[pos:]
return reshape(a, new_shape)
elif exact and extra != 0:
raise ValueError(f'{ndim=} > {nd=}')
return ary |
I'm happy to revisit the implementation.
…On Wed, Apr 17, 2024, 20:35 d.grigonis ***@***.***> wrote:
I have been using this for a fair while now. Extremely useful.
1. I would like to propose simpler implementation. The one proposed is
a bit too clever - takes me few seconds to get my head round every time I
look at it again.
def atleast_nd(ary, ndim, pos=0):
ary = array(ary, copy=False, subok=True)
if ary.ndim > ndim:
pos = normalize_axis_index(pos, ary.ndim + 1)
new_shape = list(shape)
new_shape[pos:pos] = [1] * (nd - ndim)
return reshape(a, new_shape)
return ary
2. is operator.index(ndim) needed here? Are there objects that
represent number of dimensions in this manner?
—
Reply to this email directly, view it on GitHub
<#18386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDHGMRBWZN4H2YN75GUODLY54POXAVCNFSM4XMV4ZTKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBWGI4DGNJWG44A>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
By the way, the version above is 100ns slower than yours. I chose to use this one for being slightly simpler, but I would be happy with either one merged into numpy. |
As per @charris's request, I am resurrecting #7804.
This likely also closes #12336.
Discussed in the mailing list forever ago: https://mail.python.org/pipermail/numpy-discussion/2016-July/075722.html
Discussed in the mailing list now: https://mail.python.org/pipermail/numpy-discussion/2021-February/081476.html
I did manage to implement
np.atleast_3d
in terms ofnp.atleast_nd
.