DOC: Add unravel_index examples to np.arg[min|max|sort] #9333

ElieGouzien · 2017-06-29T22:15:58Z

Hi,

Using np.arg[min|max|sort] for d-dimensional arrays whenever we want to work on the whole array, and retrieve indices is very unintuitive.
In #6078 some propositions were made to change that. But as a true modification seems not to be for tomorrow.
Here I propose a small add to the docstrings so that the user can understand by itself how to deal with it.

Élie

…m arrays.

shoyer · 2017-06-29T22:28:11Z

numpy/core/fromnumeric.py

@@ -890,6 +890,13 @@ def argsort(a, axis=-1, kind='quicksort', order=None):
    array([[0, 1],
           [0, 1]])



Can you add a short description here to explain the example? That would also be appreciated in the other examples.

What about "Retrieving global sorting indices:" ?
I'm not native English speaker so don't hesitate to propose a reformulated version.

Maybe: "Indices along each axis for sorting the flattened array"

Numpy superpowers come from the fact that the user can forget that he lives in a flat space, so I prefer avoiding a description of how it works to privilege what we get. See my new commit for my next intent (which is far from perfect).

shoyer · 2017-06-29T22:29:12Z

numpy/core/fromnumeric.py

@@ -890,6 +890,13 @@ def argsort(a, axis=-1, kind='quicksort', order=None):
    array([[0, 1],
           [0, 1]])

+    >>> np.unravel_index(np.argsort(x, axis=None), x.shape)
+    (array([0, 1, 1, 0]), array([0, 0, 1, 1]))
+    >>> x[np.unravel_index(np.argsort(x, axis=None), x.shape)]


I would probably stick to only the first line unless these additional examples show very different behavior -- space is at a premium in docstrings!

I'm not sure about it.
You are write that only the first line give indication on how argsort works.
But do the average numpy user know about advanced indexing and argument packing?
I feel like retrieving the list of indices that sort the array elements is the most basic thing we want argsort to do.

Maybe removing the second line but keeping the last one is a good option since even without knowing advanced indexing anyone can make a loop from the last line and retrieve the second line.

…m arrays: second proposition.

…indices".

ElieGouzien · 2017-06-29T23:49:03Z

With "coordinates" instead of "indices" it now sounds good to me.

eric-wieser · 2017-06-29T23:54:05Z

It was better before with indices in my opinion. "Coordinate" is a poor choice of word, especially since by convention the coordinate (x,y) is often at the index image[y,x]

ElieGouzien · 2017-06-30T00:08:37Z

That's exactly what my examples gives :
(x, y) such that a[x,y] = min(a)

For argsort it gives either:

([x1, x2, …, xk], [y1, y2, …, yk])
[(x1, y1), (x2, y2), …, (xk, yk)]
such that a[x1, y1] ≤ a[x2, y2] ≤ … ≤ a[xk, yk]

Maybe I should change the name of the array for argsort to avoid any confusion.

EDIT : the docstring of unravel_index is:

Converts a flat index or array of flat indices into a tuple of coordinate arrays.

eric-wieser · 2017-06-30T09:46:47Z

I think you misread my message. My point was that coordinate can often refer to a reversed index - (x, y) vs arr[y, x] - this is why meshgrid has two different indexing modes.

However, I agree that it appearing in the docstring of unravel_index is an argument for this naming

ElieGouzien · 2017-06-30T20:08:29Z

What I like with coordinate is that it's clearer that we have all the indices corresponding to each point.
And what about writing directly things like output : (x, y) such that a[x,y] == np.amin(a) ?

It seems that having a true option for recovering the min/max/sorting coordinates is again also asked there: #9283.

madphysicist · 2017-07-03T17:13:01Z

numpy/core/fromnumeric.py

@@ -890,6 +890,12 @@ def argsort(a, axis=-1, kind='quicksort', order=None):
    array([[0, 1],
           [0, 1]])

+    Coordinates of the sorted elements of a N-dimensional array:


Indices would be much more appropriate here, as per @eric-wieser's comment.

eric-wieser

@ElieGouzien: Please change the documentation back to say indices. np.nonzero has exactly the same return value format, and refers to it as indices. We should be consistent here.

ElieGouzien · 2017-08-05T18:10:44Z

@eric-wieser: I've applied your requested change.

eric-wieser · 2017-08-05T20:15:22Z

numpy/core/fromnumeric.py

@@ -952,6 +958,10 @@ def argmax(a, axis=None, out=None):
    >>> np.argmax(a, axis=1)
    array([2, 2])

+    Indices of the maximal elements of a N-dimensional array:
+    >>> np.unravel_index(np.argmax(a, axis=None), a.shape)
+    (1, 2)


Would be nice to show that the result of this can be used to index directly into a, and the same for the argsort example

For sure, it's committed. Let me know if you like the way I've written it.

eric-wieser · 2017-09-22T07:09:57Z

numpy/core/fromnumeric.py

+    >>> from np.testing import assert_equal
+    >>> assert_equal(x[(array([0, 1, 1, 0]), array([0, 0, 1, 1]))], np.sort(x, axis=None))
+    >>> list(zip(*np.unravel_index(np.argsort(x, axis=None), x.shape)))
+    [(0, 0), (1, 0), (1, 1), (0, 1)]


This might be clearer as:

>>> ind = np.unravel_index(np.argsort(x, axis=None), x.shape) >>> ind (array([0, 1, 1, 0]), array([0, 0, 1, 1])) >>> x[ind] # same as np.sort(x, axis=None) <whatever it is>

Repeating the call to unravel_index is obscuring the point, and assert_equal isn't that useful in docs. (we don't run doctests)

charris · 2017-10-05T18:00:47Z

Updated in #9826, so closing this. Thanks @ElieGouzien .

DOC: Add unravel_index examples to np.arg(min|max|sort)

Exemples for np.arg[min|max|sort] on how to retrieve indices for d-di…

08d8225

…m arrays.

shoyer reviewed Jun 29, 2017

View reviewed changes

ElieGouzien added 2 commits June 30, 2017 01:32

Exemples for np.arg[min|max|sort] on how to retrieve indices for d-di…

364a575

…m arrays: second proposition.

Exemples for np.arg[min|max|sort] : use of "coordinates" instead of "…

2b514e3

…indices".

eric-wieser added 04 - Documentation component: numpy._core labels Jun 29, 2017

charris changed the title ~~New exemples for np.arg[min|max|sort].~~ DOC: New exemples for np.arg[min|max|sort]. Jul 1, 2017

madphysicist reviewed Jul 3, 2017

View reviewed changes

eric-wieser requested changes Jul 30, 2017

View reviewed changes

Revert "coordinates" to "indices".

51d0710

eric-wieser reviewed Aug 5, 2017

View reviewed changes

Doc : show that the result of the exemple is directly usable as indices.

cf43db7

eric-wieser reviewed Sep 22, 2017

View reviewed changes

eric-wieser changed the title ~~DOC: New exemples for np.arg[min|max|sort].~~ DOC: Add unravel_index examples to np.arg[min|max|sort] Sep 22, 2017

charris mentioned this pull request Oct 5, 2017

DOC: Add unravel_index examples to np.arg(min|max|sort) #9826

Merged

charris closed this Oct 5, 2017

eric-wieser added a commit that referenced this pull request Oct 6, 2017

Merge pull request #9826 from charris/gh-9333

50e33f9

DOC: Add unravel_index examples to np.arg(min|max|sort)

ElieGouzien deleted the doc-arg--exemple branch October 14, 2017 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Add unravel_index examples to np.arg[min|max|sort] #9333

DOC: Add unravel_index examples to np.arg[min|max|sort] #9333

		@@ -890,6 +890,13 @@ def argsort(a, axis=-1, kind='quicksort', order=None):
		array([[0, 1],
		[0, 1]])

DOC: Add unravel_index examples to np.arg[min|max|sort] #9333

DOC: Add unravel_index examples to np.arg[min|max|sort] #9333

Conversation

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment