Skip to content

Implement structure subset views #5994

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

Closed
charris opened this issue Jun 21, 2015 · 8 comments
Closed

Implement structure subset views #5994

charris opened this issue Jun 21, 2015 · 8 comments

Comments

@charris
Copy link
Member

charris commented Jun 21, 2015

Was scheduled for 1.9, but there was a potential problem with changes in filler. If done, can probably go straight to making them writable.

@ahaldane
Copy link
Member

Do you mean views into structured arrays that only contain some of the original fields? (like a[['f0', 'f2']] = (1.0, 2.0)?)

I was working on a PR for this off and on, which is partly done. The goal is to allow writeable multi-field views, and to rework a bunch of the field assignment code (which has strange/buggy edge cases) in VOID_setitem and get_fields_transfer_function and related code.

I was going to sit on it until 1.10 is released to avoid cluttering the alpha/beta fixup process, but maybe that's not an issue.

@charris
Copy link
Member Author

charris commented Jun 26, 2015

Returning nonwritable views of subsets of structured arrays was originally scheduled for 1.9 to be followed by writable views in 1.10. Clearly, we are behind schedule ;) I had a PR for the views implemented, but a corner case test broke as missing items were filled in with spaces in the new dtype. I suspect the problem isn't fatal even if not fixable, but haven't gotten around to revisiting it.

@ahaldane
Copy link
Member

Ah, yes, that's one of the corner cases I noticed too. There's a similar one where "unviewed" bytes get filled in with garbage. Here are demos of the one you mentioned, plus that one:

>>> a = np.array([( 1, 2),( 3, 4)], dtype=[('foo', 'i8'), ('bar', 'i8')])
>>> a[:] = np.array([(10,20),(30,40)], dtype=[('bar', 'i8'),('baz', 'i8')])
>>> a
array([(0, 10), (0, 30)], 
      dtype=[('foo', '<i8'), ('bar', '<i8')])

Other edge case:

>>> a = np.array([(1,2),(3,4)], dtype=[('foo', 'i8'), ('bar', 'i8')])
>>> b = a.view({'names': ['bar'], 'formats': ['i8'], 'offsets': [8]})
>>> b[:] = 4
>>> a
array([(139714726796880, 4), (139714726796880, 4)], 
      dtype=[('foo', '<i8'), ('bar', '<i8')])

Also, I don't like the current behavior that fields get assigned "by field name", and was hoping to be able to change it to assignment "by field position", if the back-compatibility break wasn't too bad (I suspect it might not be). That way assignment by structured scalars would behave the same as assignment using tuples (or equivalently, assignment using .item() on a structured scalar).

Probably a lot of my changes (which might turn out to be a bad idea in the end..) are othogonal to your PR for making the view writeable, so I'll keep thinking about them in the meantime.

@SaurabhJha
Copy link

I am interested in starting contributing to NumPy and I would like to start with this issue. Are you people working on it? If not actively, I can take this part to let you people focus on other important things.

@charris
Copy link
Member Author

charris commented Jan 21, 2016

Thoughts? Putting this in 1.11-release for the moment, but may move to 1.12.

@charris charris modified the milestones: 1.11.0 release, 1.11.0 blockers, 1.12.0 release Jan 21, 2016
@ahaldane
Copy link
Member

Like discussed in #3641 (which is essentially the same issue?), a PR for this is #6053, but it's probably not ready for 1.11.

@ahaldane
Copy link
Member

Oh, but one thing that we might consider adding to 1.11 is the deprecation warning for #6503, implemented in #6054. I guess that's conditional on whether #6503 is the right solution.

@charris
Copy link
Member Author

charris commented Jan 21, 2016

Let's just leave it for 1.12. That should branch in 3-4 months.

ahaldane added a commit to ahaldane/numpy that referenced this issue Jun 17, 2016
This commit attempts to make structure assignment more consistent, and
then changes multi-field indices to return a view instead of a copy.

Assignment between structures now works "by field position" rather than
"by field name".

Fixes numpy#2353, fixes numpy#6085, fixes numpy#3351, fixes numpy#6085, fixes numpy#6314,
fixes numpy#2346, fixes numpy#7058, fixes numpy#3641, fixes numpy#5994, fixes numpy#7262,
fixes numpy#7493
@rgommers rgommers modified the milestone: 1.12.0 release Feb 15, 2017
@charris charris closed this as completed in 7412b85 Sep 9, 2017
theodoregoetz pushed a commit to theodoregoetz/numpy that referenced this issue Oct 23, 2017
This commit attempts to make structure assignment more consistent, and
then changes multi-field indices to return a view instead of a copy.

Assignment between structures now works "by field position" rather than
"by field name".

Fixes numpy#2353, fixes numpy#6085, fixes numpy#3351, fixes numpy#6085, fixes numpy#6314,
fixes numpy#2346, fixes numpy#7058, fixes numpy#3641, fixes numpy#5994, fixes numpy#7262,
fixes numpy#7493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants