Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

BUG: allow any axis for np.concatenate for 1D #440

Merged
merged 3 commits into from

4 participants

@matthew-brett
Collaborator

Previous numpies allowed the user to pass any integer as axis argument
to np.concatenate, when the input arrays were 1D. At some point we
tightened up on this, raising an error for axis values other than 0.
This raises a FutureWarning for axis numbers != 0, but allows them, for
backwards compatibility.

numpy/core/src/multiarray/multiarraymodule.c
@@ -337,6 +337,14 @@
if (axis < 0) {
axis += ndim;
}
+
+ if (ndim == 1 & axis != 0) {
+ PyErr_WarnEx(PyExc_FutureWarning,
+ "axis not 0 for ndim == 0; this will raise an error "
+ "in future versions of numpy", 2);
@njsmith Owner
njsmith added a note

PyErr_WarnEx isn't available in 2.4 (the Travis failure on this PR is real). Also technically this should be a DeprecationWarning, since it's only introducing stricter checking, not actually causing code to return a different result. Use the DEPRECATE macro from numpy/ndarrayobject.h to fix both of these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@njsmith njsmith commented on the diff
numpy/core/tests/test_shape_base.py
((44 lines not shown))
+ assert_array_equal(concatenate((a0, a1, a2), -1), res)
+ assert_array_equal(concatenate((a0.T, a1.T, a2.T), 0), res.T)
+
+
+def test_concatenate_sloppy0():
+ # Versions of numpy < 1.7.0 ignored axis argument value for 1D arrays. We
+ # allow this for now, but in due course we will raise an error
+ r4 = list(range(4))
+ r3 = list(range(3))
+ assert_array_equal(concatenate((r4, r3), 0), r4 + r3)
+ warnings.simplefilter('ignore', FutureWarning)
+ try:
+ assert_array_equal(concatenate((r4, r3), -10), r4 + r3)
+ assert_array_equal(concatenate((r4, r3), 10), r4 + r3)
+ finally:
+ warnings.filters.pop(0)
@njsmith Owner
njsmith added a note

Great tests, these really leave this corner of numpy better than you found it...

To be totally comprehensive, they should also check that the warning is in fact being issued by these last calls to concatenate -- see numpy/core/tests/test_multiarray.py : test_diagonal_deprecation for an example of how to do this

@matthew-brett Collaborator

Thanks - I tried another solution, baulking somewhat at the amount of copy / paste I would need to use your context manager. Maybe that could go into numpy somewhere so it could be imported? - it looks useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@87 87 referenced this pull request
Merged

Fix for issue #442 #443

@teoliphant
Owner

These are nice tests and a nice check. Will you have time to update your fork so the PR can be merged automatically?

@87 87 commented on the diff
numpy/core/src/multiarray/multiarraymodule.c
@@ -337,6 +337,16 @@
if (axis < 0) {
axis += ndim;
}
+
+ if (ndim == 1 & axis != 0) {
+ char msg[] = "axis != 0 for ndim == 1; this will raise an error in "
+ "future versions of numpy";
+ if (DEPRECATE(msg) < 0) {
+ return NULL;
+ }
+ axis = 0;
+ }
+
@87
87 added a note

Maybe this should be generalized by doing if (axis > ndim-1) --> warning message, because this issue probably exists at higher dimensions as well.

@matthew-brett Collaborator

I think the behavior has only changed for 1D arrays. For example, in numpy 1.6.1:

In [17]: a = np.zeros((1,3))

In [18]: b = np.zeros((2,3))

In [19]: np.concatenate((a, b))
Out[19]: 
array([[ 0.,  0.,  0.],
       [ 0.,  0.,  0.],
       [ 0.,  0.,  0.]])

In [20]: np.concatenate((a, b), axis=10)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
 in ()
----> 1 np.concatenate((a, b), axis=10)

ValueError: bad axis1 argument to swapaxes
@87
87 added a note

Ah, I see, the current implementation has an explicit bounds-check in concatenate, but in earlier versions it already failed for multidimensional arrays in another part of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
matthew-brett added some commits
@matthew-brett matthew-brett BUG: allow any axis for np.concatenate for 1D
Previous numpies allowed the user to pass any integer as axis argument
to np.concatenate, when the input arrays were 1D.  At some point we
tightened up on this, raising an error for axis values other than 0.
This raises a FutureWarning for axis numbers != 0, but allows them, for
backwards compatibility.
21a1d73
@matthew-brett matthew-brett BUG: change FutureWarning to DeprecationWarning
Use of PyErr_WarnEx causing failure for Python 2.4.
4475ead
@matthew-brett matthew-brett TST: test DeprecationWarning raised by concatenate
From review by Nathaniel - thanks.
69afd27
@njsmith njsmith merged commit 6a847ef into numpy:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 16, 2012
  1. @matthew-brett

    BUG: allow any axis for np.concatenate for 1D

    matthew-brett authored
    Previous numpies allowed the user to pass any integer as axis argument
    to np.concatenate, when the input arrays were 1D.  At some point we
    tightened up on this, raising an error for axis values other than 0.
    This raises a FutureWarning for axis numbers != 0, but allows them, for
    backwards compatibility.
  2. @matthew-brett

    BUG: change FutureWarning to DeprecationWarning

    matthew-brett authored
    Use of PyErr_WarnEx causing failure for Python 2.4.
  3. @matthew-brett

    TST: test DeprecationWarning raised by concatenate

    matthew-brett authored
    From review by Nathaniel - thanks.
This page is out of date. Refresh to see the latest.
View
10 numpy/core/src/multiarray/multiarraymodule.c
@@ -337,6 +337,16 @@ PyArray_ConcatenateArrays(int narrays, PyArrayObject **arrays, int axis)
if (axis < 0) {
axis += ndim;
}
+
+ if (ndim == 1 & axis != 0) {
+ char msg[] = "axis != 0 for ndim == 1; this will raise an error in "
+ "future versions of numpy";
+ if (DEPRECATE(msg) < 0) {
+ return NULL;
+ }
+ axis = 0;
+ }
+
@87
87 added a note

Maybe this should be generalized by doing if (axis > ndim-1) --> warning message, because this issue probably exists at higher dimensions as well.

@matthew-brett Collaborator

I think the behavior has only changed for 1D arrays. For example, in numpy 1.6.1:

In [17]: a = np.zeros((1,3))

In [18]: b = np.zeros((2,3))

In [19]: np.concatenate((a, b))
Out[19]: 
array([[ 0.,  0.,  0.],
       [ 0.,  0.,  0.],
       [ 0.,  0.,  0.]])

In [20]: np.concatenate((a, b), axis=10)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
 in ()
----> 1 np.concatenate((a, b), axis=10)

ValueError: bad axis1 argument to swapaxes
@87
87 added a note

Ah, I see, the current implementation has an explicit bounds-check in concatenate, but in earlier versions it already failed for multidimensional arrays in another part of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if (axis < 0 || axis >= ndim) {
PyErr_Format(PyExc_IndexError,
"axis %d out of bounds [0, %d)", orig_axis, ndim);
View
73 numpy/core/tests/test_shape_base.py
@@ -1,7 +1,7 @@
import warnings
import numpy as np
-from numpy.testing import (TestCase, assert_, assert_raises, assert_equal,
- assert_array_equal, run_module_suite)
+from numpy.testing import (TestCase, assert_, assert_raises, assert_array_equal,
+ assert_equal, run_module_suite)
from numpy.core import (array, arange, atleast_1d, atleast_2d, atleast_3d,
vstack, hstack, newaxis, concatenate)
@@ -40,6 +40,7 @@ def test_r1array(self):
assert_(atleast_1d(3.0).shape == (1,))
assert_(atleast_1d([[2,3],[4,5]]).shape == (2,2))
+
class TestAtleast2d(TestCase):
def test_0D_array(self):
a = array(1); b = array(2);
@@ -100,6 +101,7 @@ def test_3D_array(self):
desired = [a,b]
assert_array_equal(res,desired)
+
class TestHstack(TestCase):
def test_0D_array(self):
a = array(1); b = array(2);
@@ -119,6 +121,7 @@ def test_2D_array(self):
desired = array([[1,1],[2,2]])
assert_array_equal(res,desired)
+
class TestVstack(TestCase):
def test_0D_array(self):
a = array(1); b = array(2);
@@ -159,5 +162,71 @@ def test_concatenate_axis_None():
'0', '1', '2', 'x'])
assert_array_equal(r,d)
+
+def test_concatenate():
+ # Test concatenate function
+ # No arrays raise ValueError
+ assert_raises(ValueError, concatenate, ())
+ # Scalars cannot be concatenated
+ assert_raises(ValueError, concatenate, (0,))
+ assert_raises(ValueError, concatenate, (array(0),))
+ # One sequence returns unmodified (but as array)
+ r4 = list(range(4))
+ assert_array_equal(concatenate((r4,)), r4)
+ # Any sequence
+ assert_array_equal(concatenate((tuple(r4),)), r4)
+ assert_array_equal(concatenate((array(r4),)), r4)
+ # 1D default concatenation
+ r3 = list(range(3))
+ assert_array_equal(concatenate((r4, r3)), r4 + r3)
+ # Mixed sequence types
+ assert_array_equal(concatenate((tuple(r4), r3)), r4 + r3)
+ assert_array_equal(concatenate((array(r4), r3)), r4 + r3)
+ # Explicit axis specification
+ assert_array_equal(concatenate((r4, r3), 0), r4 + r3)
+ # Including negative
+ assert_array_equal(concatenate((r4, r3), -1), r4 + r3)
+ # 2D
+ a23 = array([[10, 11, 12], [13, 14, 15]])
+ a13 = array([[0, 1, 2]])
+ res = array([[10, 11, 12], [13, 14, 15], [0, 1, 2]])
+ assert_array_equal(concatenate((a23, a13)), res)
+ assert_array_equal(concatenate((a23, a13), 0), res)
+ assert_array_equal(concatenate((a23.T, a13.T), 1), res.T)
+ assert_array_equal(concatenate((a23.T, a13.T), -1), res.T)
+ # Arrays much match shape
+ assert_raises(ValueError, concatenate, (a23.T, a13.T), 0)
+ # 3D
+ res = arange(2 * 3 * 7).reshape((2, 3, 7))
+ a0 = res[..., :4]
+ a1 = res[..., 4:6]
+ a2 = res[..., 6:]
+ assert_array_equal(concatenate((a0, a1, a2), 2), res)
+ assert_array_equal(concatenate((a0, a1, a2), -1), res)
+ assert_array_equal(concatenate((a0.T, a1.T, a2.T), 0), res.T)
+
+
+def test_concatenate_sloppy0():
+ # Versions of numpy < 1.7.0 ignored axis argument value for 1D arrays. We
+ # allow this for now, but in due course we will raise an error
+ r4 = list(range(4))
+ r3 = list(range(3))
+ assert_array_equal(concatenate((r4, r3), 0), r4 + r3)
+ warnings.simplefilter('ignore', DeprecationWarning)
+ try:
+ assert_array_equal(concatenate((r4, r3), -10), r4 + r3)
+ assert_array_equal(concatenate((r4, r3), 10), r4 + r3)
+ finally:
+ warnings.filters.pop(0)
@njsmith Owner
njsmith added a note

Great tests, these really leave this corner of numpy better than you found it...

To be totally comprehensive, they should also check that the warning is in fact being issued by these last calls to concatenate -- see numpy/core/tests/test_multiarray.py : test_diagonal_deprecation for an example of how to do this

@matthew-brett Collaborator

Thanks - I tried another solution, baulking somewhat at the amount of copy / paste I would need to use your context manager. Maybe that could go into numpy somewhere so it could be imported? - it looks useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # Confurm DepractionWarning raised
+ warnings.simplefilter('always', DeprecationWarning)
+ warnings.simplefilter('error', DeprecationWarning)
+ try:
+ assert_raises(DeprecationWarning, concatenate, (r4, r3), 10)
+ finally:
+ warnings.filters.pop(0)
+ warnings.filters.pop(0)
+
+
if __name__ == "__main__":
run_module_suite()
Something went wrong with that request. Please try again.