Skip to content

Add transition code for returning view when selecting subset of fields #350

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 8 commits into from
Jul 18, 2012

Conversation

jayvius
Copy link
Contributor

@jayvius jayvius commented Jul 12, 2012

Add transition code for returning a view when selecting a subset of fields in a structured array (e.g. array[ ['f0', 'f2'] ]. Currently a copy is still returned - the WARN_ON_WRITE flag is set so a deprecation warning is triggered when writing to this copy.

Also update the WARN_ON_WRITE deprecation warning to include the above scenario. Currently the warning message only mentions the diagonal function.

@njsmith
Copy link
Member

njsmith commented Jul 13, 2012

I don't remember seeing this go by on the list -- did I miss the discussion?

@charris
Copy link
Member

charris commented Jul 13, 2012

Agree with njsmith, this needs some discussion on the list before it goes in. We also need to decide if it should be in 1.7 or 1.8.

@teoliphant
Copy link
Member

It looks like folks on the list agree this should go into 1.7 with the array diagonal change.

"by numpy.diagonal or by selecting multiple fields in a structured\n"
"array. This code will likely break in the next numpy release --\n"
"see numpy.diagonal or structured array docs for details. The quick\n"
"fix is to make an explicit copy (e.g., do arr.diagonal().copy()).";
Copy link
Member

Choose a reason for hiding this comment

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

The numpy.diagonal patch added a discussion of this to the docs for numpy.diagonal, that's what this note is referring to. Just telling people to check the structured array docs isn't useful unless there's actually some explanation of this there... and honestly I'm not even sure where the "structured array docs" are, that's pretty vague. People who get this warning message will want a much more specific explanation of what they need to do.

@njsmith
Copy link
Member

njsmith commented Jul 17, 2012

Also, this change needs tests. See test_multiarray.py:test_diagonal_deprecation for an example. (Though you probably don't have to be so thorough, since that test is also testing the WARN_ON_WRITE mechanism itself.)

@njsmith
Copy link
Member

njsmith commented Jul 17, 2012

Also, you should add an explanation of the change to doc/release/1.7.0-notes.rst

@jayvius
Copy link
Contributor Author

jayvius commented Jul 17, 2012

All good points :) As far as documentation, I think I'll add something to the arrays.indexing reference page about indexing with multiple field names for record arrays (currently it only mentions indexing with a single field name). I'll add a note here about returning a copy being deprecated in 1.7.

*e.g.* ``x[['field-name1','field-name2']]``. Currently this returns a new
array containing a copy of the values in the fields specified in the list.
As of NumPy 1.7, returning a copy is being deprecated in favor of returning
a view. A copy will continue to be returned for now, but a DeprecationWarning
Copy link
Member

Choose a reason for hiding this comment

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

This was just changed to a FutureWarning --- doc needs to be updated...

jayvius added 8 commits July 17, 2012 16:05
Add ability to get view when selecting subset of fields in a struct array,
for numpy 1.8.0. Currently a copy is still returned - set WARN_ON_WRITE flag
which will be removed in a future version of numpy.
Change WARN_ON_WRITE Deprecation Warning to include returning a copy
when selecting muultiple fields of a structured array.
- add tests for record array indexing with multiple field names
- add tests for DeprecationWarning when writing to array returned
  by record array indexing with multiple field names
@travisbot
Copy link

This pull request fails (merged 3f87c35f into 1234d1c).

@travisbot
Copy link

This pull request fails (merged a03e8b4 into 6c772fa).

teoliphant added a commit that referenced this pull request Jul 18, 2012
Add transition code for returning view when selecting subset of fields
@teoliphant teoliphant merged commit bc10053 into numpy:master Jul 18, 2012
njsmith added a commit to njsmith/numpy that referenced this pull request Jul 18, 2012
(Broken by PR numpy#350.)

Should be applied to maintenance/1.7.x as well.
@rgommers
Copy link
Member

This broke something: http://projects.scipy.org/numpy/ticket/2187

certik pushed a commit to certik/numpy that referenced this pull request Sep 12, 2012
(Broken by PR numpy#350.)

Should be applied to maintenance/1.7.x as well.
luyahan pushed a commit to plctlab/numpy that referenced this pull request Apr 25, 2024
feat: Add vqdmull[h|s][_high]_[s16|s32]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants