ENH: implement pspace assignment broadcasting #774
Conversation
In ProductSpaceElement: - Make x.parts a tuple - Re-implement __setitem__ with broadcasting - Adapt tests Closes: odlgroup#767
|
Ended up implementing broadcasting when fixing the |
| elif isinstance(indices, list): | ||
| indexed_parts = tuple(self.parts[i] for i in indices) | ||
| else: | ||
| raise TypeError('bad index type {}'.format(type(indices))) |
There was a problem hiding this comment.
Question here: This is more restrictive than __getitem__ which happily iterates over anything that allows it, including tuples. That's not Numpy-style behavior -- indexing with tuples means something very different there. (They also have indexing with arrays, but that's not applicable here)
There was a problem hiding this comment.
I agree that we should do it like you do here.
There was a problem hiding this comment.
Then I'll put similar code into __getitem__.
| @@ -602,12 +602,12 @@ class ProductSpaceElement(LinearSpaceElement): | |||
| def __init__(self, space, parts): | |||
| """Initialize a new instance.""" | |||
| super().__init__(space) | |||
| self._parts = list(parts) | |||
| self.__parts = tuple(parts) | |||
| elif isinstance(indices, list): | ||
| indexed_parts = tuple(self.parts[i] for i in indices) | ||
| else: | ||
| raise TypeError('bad index type {}'.format(type(indices))) |
There was a problem hiding this comment.
I agree that we should do it like you do here.
| else: | ||
| # `values` is iterable; it could still represent a single | ||
| # element of a power space. | ||
| if self.space.is_power_space and values in self.space[0]: |
There was a problem hiding this comment.
What about stuff like assigning a list? I think that works in rn, i.e.
>>> r2 = odl.rn(2)
>>> el = r2.element()
>>> el[:] = [1, 2]
don't we want something like that to work here?
There was a problem hiding this comment.
|
Finally did this last (trivial) change, merge after CI. Edit: Bug found, needs fix first. |
In
ProductSpaceElement:__setitem__with broadcastingCloses: #767