Skip to content

DEP,BUG: Coercion/cast of array to a subarray dtype will be fixed - #17596

Merged
mattip merged 1 commit into
numpy:masterfrom
seberg:subarray-dtype-futurewarning
Oct 29, 2020
Merged

mattip merged 1 commit into
numpy:masterfrom
seberg:subarray-dtype-futurewarning

Conversation

@seberg

@seberg seberg commented Oct 20, 2020

Copy link
Copy Markdown
Member

This currently appends the subarray dtype dimensions first and then
tries to assign to the result array which uses incorrect broadcasting
(broadcasting against the subarray dimensions instead of repeating
each element according to the subarray dimensions).

This also fixes the python scalar pathway np.array(2, dtype="(2)f4,")
which previously only filled the first value. I consider that a clear
bug fix.

Closes gh-17511

@seberg
seberg force-pushed the subarray-dtype-futurewarning branch from 760be52 to 81b8611 Compare October 20, 2020 21:47
Comment thread numpy/core/tests/test_dtype.py Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To be clear, the first case here previously returned [3, uninitialized, uninitialized], while the second case actually did the right thing. So I considered that a simple bugfix here.

@seberg seberg added this to the 1.20.0 release milestone Oct 20, 2020
Comment thread doc/release/upcoming_changes/17596.future.rst Outdated
@mhvk

mhvk commented Oct 21, 2020

Copy link
Copy Markdown
Contributor

@seberg - this looks quite good. With this change, will np.array([], '2f4') still raise a FutureWarning? I think it doesn't have to, since the result will not change in the future, but I didn't see a test to check that (and didn't quite follow the code close enough to see if this special case was caught).

This currently appends the subarray dtype dimensions first and then
tries to assign to the result array which uses incorrect broadcasting
(broadcasting against the subarray dimensions instead of repeating
each element according to the subarray dimensions).

This also fixes the python scalar pathway `np.array(2, dtype="(2)f4,")`
which previously only filled the first value.  I consider that a clear
bug fix.

Closes numpygh-17511
@seberg
seberg force-pushed the subarray-dtype-futurewarning branch from 22a10b1 to fcc3940 Compare October 21, 2020 17:39
@seberg

seberg commented Oct 21, 2020

Copy link
Copy Markdown
Member Author

Thanks for taking a look! I am scanning the full input for array(-like) objects in the code, it is a bit annoying, but feel this is fringe enough that it is OK. Since I look for array likes, it will not trigger for empty sequences.

There are cases where a warning will be given even though nothing will change, e.g. np.array(3).astype("(4)f4") has the same future behaviour as with the current broadcast behaviour.

@mhvk

mhvk commented Oct 21, 2020

Copy link
Copy Markdown
Contributor

OK, that sounds good! And I can see that you cannot do everything... But perhaps good to add a test for the empty list case, just in case?

@seberg

seberg commented Oct 21, 2020

Copy link
Copy Markdown
Member Author

Oh, seems I accidentally deleted that sentence snippet: I had just added the test.

@seberg seberg closed this Oct 21, 2020
@seberg seberg reopened this Oct 21, 2020
@mattip
mattip merged commit 08f9eeb into numpy:master Oct 29, 2020
@mattip

mattip commented Oct 29, 2020

Copy link
Copy Markdown
Member

Thanks @seberg

@seberg
seberg deleted the subarray-dtype-futurewarning branch October 29, 2020 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Initializing arrays with subarray dtype

3 participants