Skip to content

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

Merged
merged 1 commit into from
May 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/release/1.13.0-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ Deprecations
* Use of the C-API ``NPY_CHAR`` type number deprecated since version 1.7 will
now raise deprecation warnings at runtime. Extensions built with older f2py
versions need to be recompiled to remove the warning.
* ``np.ma.argsort`` should be called with an explicit `axis` argument when
applied to arrays with more than 2 dimensions, as the default value of
this argument (``None``) is inconsistent with the rest of numpy (``-1``).


Build System Changes
Expand Down
50 changes: 46 additions & 4 deletions numpy/ma/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

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, not 2, as it is not directly in the deprecated function

return None


def doc_note(initialdoc, note):
"""
Expand Down Expand Up @@ -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
Expand All @@ -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.
Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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)
Expand Down
41 changes: 41 additions & 0 deletions numpy/ma/tests/test_deprecations.py
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 """
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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()