Broadcasting not working with record array with a single record #6085

Activity

1. ahaldane commented on Jul 16, 2015

   

   ahaldane

   on Jul 16, 2015

   Member

   More actions

   Hmm, this is because the first example goes through VOID_setitem, while the second example goes through dtype_transfer code. (the split happens here). The two paths differ in behavior, and only the dtype_tranfer path allows assignment of a scalar to a structured element.

   I am working on a PR to overhaul structure assignmet (#6053), and I had noticed the two paths differ (see "other changes"). I had thought it was intentional, but this issue makes me think it the two paths need to be identical. Thanks for bringing it up!
2. dashesy commented on Jul 16, 2015

   

   dashesy

   on Jul 16, 2015

   Author

   More actions

   The structure assignment a[['p','pr']]=(1,2) is actually interesting, I tried that once and it did not work of course. If it works it would be like a light-weight Pandas, and therefore quite useful.
3. seberg commented on Jul 16, 2015

   

   seberg

   on Jul 16, 2015

   Member

   More actions

   It might be going too far, but to me the whole concept of setitem on a (void) scalar is dubious at best (I think scalars should be immutable). On first sight, I could guess it was implemented just to allow
   arr[0]['field'] = 1 be identical to arr['field'][0] = 1. If it currently is pretty much utterly useless, I would throw in to consider just deprecating it entirely. Also not sure that broadcasting of the fields itself is necessary, considering that fields might just as well be completely different types.
4. ahaldane commented on Jul 16, 2015

   

   ahaldane

   on Jul 16, 2015

   Member

   More actions

   @seberg You are referring to setfield rather than setitem, right? VOID_setitem is used to set individual elements of an array of void type, and is unrelated to being able to modify individual fields of a structure (which is performed by get/set_field). I think you are arguing that voidtype_setfield should not exist, a somewhat different issue from the one here. (And I disagree :) - why should arr[0]['field'] = 1 behave differently from arr[:1]['field'] = 1? )

   As to "broadcasting of the fields", I agree. Assignment of an unstructured scalar to a structure array might be worth deprecating. That is, in

   >>> a = np.array([(1,2),(3,4)], dtype='i8,i8')
   >>> a[:] = 1
   >>> a[:] = (1,1)
   

   the first assignment should raise a typerror, and really only the second assignment should be allowed. In my first pass of #6053 I tried this (easy to do), but it turns out a fair amount of code will break, so I'm keeping it for now.
5. seberg commented on Jul 17, 2015

   

   seberg

   on Jul 17, 2015

   Member

   More actions

   I actually had to think a bit to support my gut feeling with argument ;). The thing is, that I think there are two "usual" ways:

   1. Immutability:

   a = np.array([1])
   scalar = a[0]
   scalar += 1
   print(a)  # unchanged
   

   2. Mutable (really only python objects). Using dict for similarity to structured types:

   a = np.array([{'a': 1, 'b': 2}])
   b = np.array([{'a': 1, 'b': 2}])
   scalar = a[0]
   b[0] = scalar
   scalar['a'] = 3
   # Both changed:
   print(a)
   print(b)
   

   However, structured types do neither:

   a = np.array([(1, 2)], dtype=[('a', np.int_), ('b', np.int_)])
   b = np.array([(1, 2)], dtype=[('a', np.int_), ('b', np.int_)])
   scalar = a[0]
   b[0] = scalar
   scalar['a'] = 3
   print(a)  # changed
   print(b)  # not changed.
   

   So that is why, unless there is some good reason to introduce a new class of types in some sense, I would say that our scalars should in general be immutable (unless they are references to -- such as, though not necessarily I guess, python objects -- mutable types).
6. ahaldane commented on Jul 17, 2015

   

   ahaldane

   on Jul 17, 2015

   Member

   More actions

   Hmm I see your point.

   I guess in my mind structured arrays behave a little like they have an extra dimension, such that indexing just the first dimension gives you a view. That is, the 1d structured array b below behaves a like the 2d array a:

   >>> a = ones((2,3), dtype='i8')
   >>> a[0][1] += 1
   >>> a
   array([[1, 2, 1],
          [1, 1, 1]])
   
   >>> b = ones((2,), dtype='i8,i8,i8')
   >>> b[0]['f1'] += 1
   >>> b
   array([(1, 2, 1), (1, 1, 1)], 
         dtype=[('f0', '<i8'), ('f1', '<i8'), ('f2', '<i8')])
   

   In fact in #6053 I first tried making structured arrays have an extra dimension (so b would have shape (2,3)), but it made the broadcasting rules too complicated so I removed it. But it was tantalizing to be able to do

   >>> b[0] = zeros(3)
   

   (I have a way to effectively allow this which I'll propose if #6053 goes through, which involves doing b[0] = structured(zeros(3)) instead)

   I'll think about it.
7. seberg commented on Jul 17, 2015

   

   seberg

   on Jul 17, 2015

   Member

   More actions

   Good point. Maybe then the question could be why we have structured scalars ;), if you think of them having an extra dimension, indexing should maybe return a 0-d array for them.
   I guess you could be right that this is how it was intended, I still prefer to think of them as elements (since it is encoded in the dtype), but I admit I can't reject the extra dimension idea outright and guess it could be a common idea.
8. seberg commented on Jul 17, 2015

   

   seberg

   on Jul 17, 2015

   Member

   More actions

   (philosophizing here: this could be an argument for that "datashape" idea ^^, you still got atomic dtypes, but the structured part could have a place in the shape in some sense, though I still doubt there is much point in going down that road for us)
9. ahaldane

   added 3 commits that reference this issue on Jun 17, 2016

   MAINT: improve struct assignment, allow multi fields views

   ...

   This issue will close once commit 2ef0103 is merged into the 'master' branch.

   2ef0103
10. 5 remaining items

    Load more
11. charris

    closed this as completedin 7412b85on Sep 9, 2017
12. theodoregoetz

    added a commit that references this issue on Oct 23, 2017

    MAINT: improve struct assignment, allow multi fields views

    ...

    This issue will close once commit ae582a8 is merged into the 'master' branch.

    ae582a8