BUG: Fixed a bug with string representation of masked structured arrays #6094
Conversation
|
For non-masked arrays, the row of a structured array column might look like e.g. this is with a 2x2x2 cell after a scalar cell. So the i.e the commas are missing. Do we care about having exactly consistent output compared to the non-masked arrays? If so, what would be the best way to obtain that? (alternatively, we could get rid of the commas in the non-masked output, in my opinion it looks cleaner without) |
|
Can any |
|
I'm no I didn't try it myself, but is there a reason you didn't do the same thing as the |
|
@ahaldane - ah yes, I think that will help. I will implement this later today. |
|
Sorry for the delay, will get back to this soon. |
cae91ab to
e0b2e8c
Compare
|
Could someone review this? This is currently causing issues in Astropy so it would be nice to have it fixed in Numpy 1.11 - thanks! |
| p = str(p) | ||
| r = [(str(_), p)[int(_m)] for (_, _m) in zip(self._data.tolist(), m)] | ||
| return "(%s)" % ", ".join(r) | ||
| __repr__ = __str__ |
There was a problem hiding this comment.
Hmm, doesn't this lose something? What is thinking here?
There was a problem hiding this comment.
@charris - well, the output was basically the same before between the two. This is backward-compatible for the cases that did work.
There was a problem hiding this comment.
@charris - would you prefer to see this behave differently? If so, do you have suggestions for expected behavior?
There was a problem hiding this comment.
It seems OK to me, since it looks like what normal void-scalars do too (repr is the same as str).
>>> a = np.array([([1, 2, 3],)], dtype=[('a', '<i8', (3,))])
>>> repr(a[0])
'([1, 2, 3],)'
This fixes a bug that caused the string representation of masked structured array rows with multi-dimensional columns to fail (numpy#6019), and includes a regression test. Since __repr__ suffered from a similar bug, and since previously __repr__ returned the same as __str__ for mvoid, we now set __repr__ to reference the same method as __str__.
|
@ahaldane - thanks for your suggestion, it works great! I've updated the pull request. I also added a test for the nested case you showed. |
|
Cool, this looks ready to me now. @charris any further comments? |
|
@ahaldane I'm good, leaving this to you :) |
|
All right, merging. Thanks a lot, @astrofrog ! |
BUG: Fixed a bug with string representation of masked structured arrays
|
Should this go into maintenance/1.10.x? |
|
Could do. |
This is a fix for the issue reported in #6019 and includes a regression test.
One thing I am not sure about is what the string representation should look like if the column is e.g. 2-dimensional - at the moment it will include a line return, but I'm not sure if that's desirable? I could strip out the line returns in that case?