-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
DEP: deprecate calling ma.argsort without an axis #8918
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,6 +93,33 @@ | |
class MaskedArrayFutureWarning(FutureWarning): | ||
pass | ||
|
||
def _deprecate_argsort_axis(arr): | ||
""" | ||
Adjust the axis passed to argsort, warning if necessary | ||
|
||
Parameters | ||
---------- | ||
arr | ||
The array which argsort was called on | ||
|
||
np.ma.argsort has a long-term bug where the default of the axis argument | ||
is wrong (gh-8701), which now must be kept for backwards compatibiity. | ||
Thankfully, this only makes a difference when arrays are 2- or more- | ||
dimensional, so we only need a warning then. | ||
""" | ||
if arr.ndim <= 1: | ||
# no warning needed - but switch to -1 anyway, to avoid surprising | ||
# subclasses, which are more likely to implement scalar axes. | ||
return -1 | ||
else: | ||
# 2017-04-11, Numpy 1.13.0, gh-8701: warn on axis default | ||
warnings.warn( | ||
"In the future the default for argsort will be axis=-1, not the " | ||
"current None, to match its documentation and np.argsort. " | ||
"Explicitly pass -1 or None to silence this warning.", | ||
MaskedArrayFutureWarning, stacklevel=3) | ||
return None | ||
|
||
|
||
def doc_note(initialdoc, note): | ||
""" | ||
|
@@ -5285,7 +5312,7 @@ def round(self, decimals=0, out=None): | |
out.__setmask__(self._mask) | ||
return out | ||
|
||
def argsort(self, axis=None, kind='quicksort', order=None, | ||
def argsort(self, axis=np._NoValue, kind='quicksort', order=None, | ||
endwith=True, fill_value=None): | ||
""" | ||
Return an ndarray of indices that sort the array along the | ||
|
@@ -5295,8 +5322,15 @@ def argsort(self, axis=None, kind='quicksort', order=None, | |
Parameters | ||
---------- | ||
axis : int, optional | ||
Axis along which to sort. The default is -1 (last axis). | ||
If None, the flattened array is used. | ||
Axis along which to sort. If None, the default, the flattened array | ||
is used. | ||
|
||
.. versionchanged:: 1.13.0 | ||
Previously, the default was documented to be -1, but that was | ||
in error. At some future date, the default will change to -1, as | ||
originally intended. | ||
Until then, the axis should be given explicitly when | ||
``arr.ndim > 1``, to avoid a FutureWarning. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the comment to the axis argument |
||
kind : {'quicksort', 'mergesort', 'heapsort'}, optional | ||
Sorting algorithm. | ||
order : list, optional | ||
|
@@ -5341,6 +5375,10 @@ def argsort(self, axis=None, kind='quicksort', order=None, | |
|
||
""" | ||
|
||
# 2017-04-11, Numpy 1.13.0, gh-8701: warn on axis default | ||
if axis is np._NoValue: | ||
axis = _deprecate_argsort_axis(self) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing the test here makes it more obvious that this only affects default arguments |
||
|
||
if fill_value is None: | ||
if endwith: | ||
# nan > inf | ||
|
@@ -6560,10 +6598,14 @@ def power(a, b, third=None): | |
argmin = _frommethod('argmin') | ||
argmax = _frommethod('argmax') | ||
|
||
def argsort(a, axis=None, kind='quicksort', order=None, endwith=True, fill_value=None): | ||
def argsort(a, axis=np._NoValue, kind='quicksort', order=None, endwith=True, fill_value=None): | ||
"Function version of the eponymous method." | ||
a = np.asanyarray(a) | ||
|
||
# 2017-04-11, Numpy 1.13.0, gh-8701: warn on axis default | ||
if axis is np._NoValue: | ||
axis = _deprecate_argsort_axis(a) | ||
|
||
if isinstance(a, MaskedArray): | ||
return a.argsort(axis=axis, kind=kind, order=order, | ||
endwith=endwith, fill_value=fill_value) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
"""Test deprecation and future warnings. | ||
|
||
""" | ||
from __future__ import division, absolute_import, print_function | ||
|
||
import numpy as np | ||
from numpy.testing import TestCase, run_module_suite, assert_warns | ||
from numpy.ma.testutils import assert_equal | ||
|
||
|
||
class TestArgsort(TestCase): | ||
""" gh-8701 """ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should include date and numpy version, that helps keep track of expiration dates, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought that having it repeated in the tests would just be noise - date and version are already in the source |
||
def _test_base(self, argsort, cls): | ||
arr_0d = np.array(1).view(cls) | ||
argsort(arr_0d) | ||
|
||
arr_1d = np.array([1, 2, 3]).view(cls) | ||
argsort(arr_1d) | ||
|
||
# argsort has a bad default for >1d arrays | ||
arr_2d = np.array([[1, 2], [3, 4]]).view(cls) | ||
result = assert_warns( | ||
np.ma.core.MaskedArrayFutureWarning, argsort, arr_2d) | ||
assert_equal(result, argsort(arr_2d, axis=None)) | ||
|
||
# should be no warnings for explictly specifiying it | ||
argsort(arr_2d, axis=None) | ||
argsort(arr_2d, axis=-1) | ||
|
||
def test_function_ndarray(self): | ||
return self._test_base(np.ma.argsort, np.ndarray) | ||
|
||
def test_function_maskedarray(self): | ||
return self._test_base(np.ma.argsort, np.ma.MaskedArray) | ||
|
||
def test_method(self): | ||
return self._test_base(np.ma.MaskedArray.argsort, np.ma.MaskedArray) | ||
|
||
|
||
if __name__ == "__main__": | ||
run_module_suite() |
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.
This level needed to be
3
, not2
, as it is not directly in the deprecated function