DOC: v/h/dstack docstr shouldn't imply deprecation #10057
Conversation
86fb59c to
744651a
Compare
numpy/core/shape_base.py
Outdated
| Take a sequence of arrays and stack them horizontally to make | ||
| a single array. Rebuild arrays divided by `hsplit`. | ||
| This is equivalent to concatenation along the second dimension for 2d and | ||
| 3d arrays, or along the first dimension for 1d arrays. Rebuilds arrays |
There was a problem hiding this comment.
For number of dimensions > 2 it always stacks along second dimension. This seems to imply that it only works for arrays ndim <= 3.
numpy/lib/shape_base.py
Outdated
| This function continues to be supported for backward compatibility, but | ||
| you should prefer ``np.concatenate`` or ``np.stack``. The ``np.stack`` | ||
| function was added in NumPy 1.10. | ||
| This function is meant for use on arrays with 3 dimensions or less. The |
There was a problem hiding this comment. 8000
Not sure on the 3-D or less description here. It might not seem sensible for more dimensions, but there is no restriction. Maybe "makes most sense".
|
It might be worth mentioning that the 3d cases use h,v, and d as they might be referred to in a multichannel image. They don't correspond to the meanings expected of a matrix stack. |
544fabc to
676881f
Compare
|
Updated, to use |
|
Thanks, I like this formulation better. |
|
I think to justify "makes sense for 3d" in all the cases, you need to give the image example - not just for |
aa46a95 to
9875da6
Compare
|
Added the image example to all docstrings. |
numpy/core/shape_base.py
Outdated
| This function continues to be supported for backward compatibility, but | ||
| you should prefer ``np.concatenate`` or ``np.stack``. The ``np.stack`` | ||
| function was added in NumPy 1.10. | ||
| This function makes most sense for arrays with 3 dimensions or less. For |
There was a problem hiding this comment.
Should be "fewer", not "less"
There was a problem hiding this comment.
No, "less" is correct and preferred in the "N items or less" construction.
There was a problem hiding this comment.
The comments on the blog post seem pretty divided, but it's not worth arguing over here - a poll of my coworkers reveals this to be a divisive case!
Having said that, I think "up to 3 dimensions" would read better than either of those. But this patch is good enough without that.
numpy/core/shape_base.py
Outdated
| This function makes most sense for arrays with 3 dimensions or less. For | ||
| instance, for image pixel-data with a height (first axis), width (second | ||
| axis), and possibly r/g/b channels (third axis). The functions | ||
| ``np.concatenate``, ``np.stack`` and ``np.block`` provide more generalized |
There was a problem hiding this comment.
Do we want to use single `s here so that these generate links?
9875da6 to
e764934
Compare
numpy/core/shape_base.py
Outdated
| @@ -183,23 +183,25 @@ def vstack(tup): | |||
| """ | |||
| Stack arrays in sequence vertically (row wise). | |||
|
|
|||
| Take a sequence of arrays and stack them vertically to make a single | |||
| array. Rebuild arrays divided by `vsplit`. | |||
| This is equivalent to concatenation along the first axis after 1d arrays of | |||
There was a problem hiding this comment.
I think we settled on the form 1-D for these things, although a grep shows some leftovers.
numpy/core/shape_base.py
Outdated
| Tuple containing arrays to be stacked. The arrays must have the same | ||
| shape along all but the first axis. | ||
| The arrays must have the same shape along all but the first axis. | ||
| 1d arrays must have the same length. |
e764934 to
e1811e1
Compare
|
Fixed to use |
|
Merged ignoring irrelevant appveyor tests. Thanks Allan. |
This changes the
v/h/dstackdocstrings to more clearly describe what they do, and avoids implying they are deprecated.This change was discussed on the mailing list here:
https://mail.python.org/pipermail/numpy-discussion/2017-November/077391.html