Skip to content

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

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.

Sign up for GitHub

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 3 commits into from
Sep 24, 2012
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
21a1d73
BUG: allow any axis for np.concatenate for 1D
matthew-brett Sep 13, 2012
4475ead
BUG: change FutureWarning to DeprecationWarning
matthew-brett Sep 13, 2012
69afd27
TST: test DeprecationWarning raised by concatenate
matthew-brett Sep 13, 2012
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
10 changes: 10 additions & 0 deletions numpy/core/src/multiarray/multiarraymodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (axis < 0 || axis >= ndim) {
PyErr_Format(PyExc_IndexError,
"axis %d out of bounds [0, %d)", orig_axis, ndim);
Expand Down
73 changes: 71 additions & 2 deletions numpy/core/tests/test_shape_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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