Skip to content

DEP: Deprecate collapsing shape-1 dtype fields to scalars. #13326

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
eric-wieser merged 1 commit into from
May 26, 2019
Merged

DEP: Deprecate collapsing shape-1 dtype fields to scalars. #13326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 doc/release/1.17.0-notes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ a manner are very rare in practice and only available through the NumPy C-API.
Future Changes
==============

Shape-1 fields in dtypes won't be collapsed to scalars in a future version
--------------------------------------------------------------------------

Currently, a field specified as ``[(name, dtype, 1)]`` or ``"1type"`` is
interpreted as a scalar field (i.e., the same as ``[(name, dtype)]`` or
``[(name, dtype, ()]``). This now raises a FutureWarning; in a future version,
it will be interpreted as a shape-(1,) field, i.e. the same as ``[(name,
dtype, (1,))]`` or ``"(1,)type"`` (consistently with ``[(name, dtype, n)]``
/ ``"ntype"`` with ``n>1``, which is already equivalent to ``[(name, dtype,
(n,)]`` / ``"(n,)type"``).

Expired deprecations
====================
Expand Down
25 changes: 16 additions & 9 deletions numpy/core/src/multiarray/descriptor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,22 @@ _convert_from_tuple(PyObject *obj, int align)
"invalid shape in fixed-type tuple.");
goto fail;
}
/*
* If (type, 1) was given, it is equivalent to type...
* or (type, ()) was given it is equivalent to type...
*/
if ((shape.len == 1
&& shape.ptr[0] == 1
&& PyNumber_Check(val))
|| (shape.len == 0
&& PyTuple_Check(val))) {
/* if (type, ()) was given it is equivalent to type... */
if (shape.len == 0 && PyTuple_Check(val)) {
npy_free_cache_dim_obj(shape);
return type;
}
/* (type, 1) use to be equivalent to type, but is deprecated */
if (shape.len == 1
&& shape.ptr[0] == 1
&& PyNumber_Check(val)) {
/* 2019-05-20, 1.17 */
if (DEPRECATE_FUTUREWARNING(
Comment thread
eric-wieser marked this conversation as resolved.
Resolved
"Passing (type, 1) or '1type' as a synonym of type is "
"deprecated; in a future version of numpy, it will be "
"understood as (type, (1,)) / '(1,)type'.") < 0) {
goto fail;
}
npy_free_cache_dim_obj(shape);
return type;
}
Expand Down
8 changes: 8 additions & 0 deletions numpy/core/tests/test_deprecations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,11 @@ def test_get_numeric_ops(self):
# other tests.
self.assert_deprecated(np.set_numeric_ops, kwargs={})
assert_raises(ValueError, np.set_numeric_ops, add='abc')


class TestShape1Fields(_DeprecationTestCase):
warning_cls = FutureWarning
Comment thread
eric-wieser marked this conversation as resolved.
Resolved

# 2019-05-20, 1.17.0
def test_shape_1_fields(self):
self.assert_deprecated(np.dtype, args=([('a', int, 1)],))
7 changes: 5 additions & 2 deletions numpy/core/tests/test_dtype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,10 @@ def test_nonequivalent_record(self):
def test_shape_equal(self):
"""Test some data types that are equal"""
assert_dtype_equal(np.dtype('f8'), np.dtype(('f8', tuple())))
assert_dtype_equal(np.dtype('f8'), np.dtype(('f8', 1)))
# FutureWarning during deprecation period; after it is passed this
# should instead check that "(1)f8" == "1f8" == ("f8", 1).
with pytest.warns(FutureWarning):
Comment thread
eric-wieser marked this conversation as resolved.
Resolved
assert_dtype_equal(np.dtype('f8'), np.dtype(('f8', 1)))
assert_dtype_equal(np.dtype((int, 2)), np.dtype((int, (2,))))
assert_dtype_equal(np.dtype(('<f4', (3, 2))), np.dtype(('<f4', (3, 2))))
d = ([('a', 'f4', (1, 2)), ('b', 'f8', (3, 1))], (3, 2))
Expand Down Expand Up @@ -447,7 +450,7 @@ def test_shape_invalid(self):

def test_alignment(self):
#Check that subarrays are aligned
t1 = np.dtype('1i4', align=True)
t1 = np.dtype('(1,)i4', align=True)
t2 = np.dtype('2i4', align=True)
Comment thread
eric-wieser marked this conversation as resolved.
Resolved
assert_equal(t1.alignment, t2.alignment)

Expand Down
2 changes: 1 addition & 1 deletion numpy/core/tests/test_numeric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def test_zeroresize(self):

def test_reshape_from_zero(self):
# See also gh-6740
A = np.zeros(0, dtype=[('a', np.float32, 1)])
A = np.zeros(0, dtype=[('a', np.float32)])
Ar = np.resize(A, (2, 1))
assert_array_equal(Ar, np.zeros((2, 1), Ar.dtype))
assert_equal(A.dtype, Ar.dtype)
Expand Down
2 changes: 1 addition & 1 deletion numpy/core/tests/test_scalarbuffer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def test_void_scalar_structured_data(self):
assert_(isinstance(x, np.void))
mv_x = memoryview(x)
expected_size = 16 * np.dtype((np.unicode_, 1)).itemsize
expected_size += 2 * np.dtype((np.float64, 1)).itemsize
expected_size += 2 * np.dtype(np.float64).itemsize
assert_equal(mv_x.itemsize, expected_size)
assert_equal(mv_x.ndim, 0)
assert_equal(mv_x.shape, ())
Expand Down
3 changes: 2 additions & 1 deletion numpy/ma/mrecords.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ def __getattribute__(self, attr):
except IndexError:
# Couldn't find a mask: use the default (nomask)
pass
hasmasked = _mask.view((bool, (len(_mask.dtype) or 1))).any()
tp_len = len(_mask.dtype)
hasmasked = _mask.view((bool, ((tp_len,) if tp_len else ()))).any()

@eric-wieser eric-wieser May 25, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This if condition should be _mask.dtype.fields is None, else it blows up on structured types with 0 fields. Arguably out of scope, but may as well fix this.

If you're looking for another deprecation for another PR, len(dtype) is looking like a bad idea on non structured dtypes... If not, I'll probably do that myself.

@eric-wieser eric-wieser May 25, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although actually the function is crap anyway and should be using one of the recursive helpers in ma/core that ors together all the fields

@anntzer anntzer May 25, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So should I update this PR, or just leave it as it is and let you do a followup? (I would prefer the latter :))

@eric-wieser eric-wieser May 25, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose you could argue that this PR is about fixing things for length-1 subarrays, and you've succeeded even without fixing this. I think fine to put in as is, assuming we can sort out the ci failure by restarting

if (obj.shape or hasmasked):
obj = obj.view(MaskedArray)
obj._baseclass = ndarray
Expand Down
3 changes: 1 addition & 2 deletions numpy/random/tests/test_random.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,7 @@ def test_shuffle(self):
.view(np.recarray)),
# gh-4270
lambda x: np.asarray([(i, i) for i in x],
[("a", object, 1),
("b", np.int32, 1)])]:
[("a", object), ("b", np.int32)])]:
np.random.seed(self.seed)
alist = conv([1, 2, 3, 4, 5, 6, 7, 8, 9, 0])
np.random.shuffle(alist)
Expand Down