added reverse arg in methods indicesofsorted and labelsofsorted (issue 490) #493
Conversation
| * added a feature (see the :ref:`miscellaneous section <misc>` for details). | ||
|
|
||
| * added `reverse` argument to methods `indicesofsorted` and `labelsofsorted`. | ||
| When used values are sorted in descending order instead of ascending (default behavior): |
There was a problem hiding this comment.
s/.\nWhen used values are sorted/ to sort values/ ?
larray/core/array.py
Outdated
| FR F M | ||
| IT M F | ||
| >>> arr.labelsofsorted(X.sex, reverse=True) | ||
| nat\\sex 1 0 |
There was a problem hiding this comment.
I don't think this is the result we want. At least that was not what I needed when I wanted reverse: I expected:
nat\\sex 0 1
BE F M
FR M F
IT F MThe reasoning is this: I want to sort from highest to lowest, and given that sort, I want to know which is the first element (ie indice 0), which is second(1), etc. Not just display them in flipped order.
larray/core/array.py
Outdated
| 2 1 0 | ||
| >>> arr.indicesofsorted(X.nat, reverse=True) | ||
| nat\\sex M F | ||
| 2 1 0 |
larray/core/array.py
Outdated
| posargmax = renamed_to(indexofmax, 'posargmax') | ||
|
|
||
| def labelsofsorted(self, axis=None, kind='quicksort'): | ||
| def labelsofsorted(self, axis=None, kind='quicksort', reverse=False): |
There was a problem hiding this comment.
I wonder if reverse shouldn't be the second argument, as it is much more likely to be used than kind.
larray/core/array.py
Outdated
| axis = self.axes[0] | ||
| axis, axis_idx = self.axes[axis], self.axes.index(axis) | ||
| data = self.data.argsort(axis_idx, kind=kind) | ||
| data = np.argsort(-1 * self.data if reverse else self.data, axis_idx, kind=kind) |
There was a problem hiding this comment.
Why not use something like this:
data = self.data.argsort(axis_idx, kind=kind)
if reverse:
reverser = tuple(slice(None, None, -1) if i == axis_idx else slice(None)
for i in range(self.ndim))
data = data[reverser]this wouldn't make a copy of the data.
| if axis is None: | ||
| if self.ndim > 1: | ||
| raise ValueError("array has ndim > 1 and no axis specified for posargsort") | ||
| raise ValueError("array has ndim > 1 and no axis specified for indicesofsorted") |
There was a problem hiding this comment.
Nice catch! The corresponding message in labelsofsorted needs to be fixed too, though.
larray/core/array.py
Outdated
| kind : {'quicksort', 'mergesort', 'heapsort'}, optional | ||
| Sorting algorithm. Defaults to 'quicksort'. | ||
|
Wainting for 0.26.1 release before to merge |
…rted and indicesofsorted
alixdamman
force-pushed
the
reverse_arg_490
branch
from
0f68054 to
22ae6ac
Compare
|
can be merged now |
2 participants
No description provided.