-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
BUG: fix _array2string for structured array (issue #5692) #8160
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
Conversation
@skwbc You should expand the commit message to contain your comment above. Also include |
@charris |
for descr in dtype_.descr: | ||
field_name = descr[0] | ||
field_values = data[field_name] | ||
if 1 < len(field_values.shape): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this if would be clearer written as
if len(field_values.shape) <= 1:
blah
else:
blah
As one expects to handle things in order of increasing size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote it.
field_name = descr[0] | ||
field_values = data[field_name] | ||
if 1 < len(field_values.shape): | ||
format_function = repr_format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is repr_format
the correct choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, yes (low confidence).
By using repr_format
, this updated _array2string
should be called and it works fine.
LGTM. @ahaldane Might take a look. |
The cause of issue #5692 is that `_array2string` (in numpy/core/arrayprint.py) doesn’t have format function for structured arrays and it uses general purpose format function to format array elements of structured arrays. This commit adds `StructureFormat` class to format structured array elements. `_get_format_function` instantiates `StructureFormat` by instantiating format function classes for each field of structure recursively and merge them. Closes #5692.
Thanks @skwbc . If you want to take a shot at adding standard docstrings to the |
Sorry, I only just saw this. Looks like the right idea, but I see two problems: First, this will raise an exception if the dtype has padding bytes:
That's easy to fix though, just do
instead of
This happens because Second, I don't think this treats subarrays quite correctly:
but I think it should give (like in 1.11)
@skwbc if you can make a new PR with to fix those, please do. If you don't in a few days one of us will. |
@ahaldane I'll make a new PR in a few days. |
FWIW this broke some doctests I had. So is complicating my ability to test against 1.11 and 1.12. Is there a way to fix this to keeping a trailing zero or force dropping it in 1.11? |
Perhaps write your doctests as
Instead of relying on the repr. Alternatively, only run your doctests on one version of numpy? |
Is there no option to specify how I would like to format floats? It seems like this changed how formatting is handled under the surface. Would expect that the users would have some recourse. Is that not the case? |
@jakirkham - It is indeed quite annoying and I had similar problems adjusting the doctests for astropy; my conclusion was that it was better to mimic new numpy even on old versions, but the problem is that one used to have no control over the formatting. So, I ended up writing code such that it used an updated |
Thanks for the tips, @mhvk. Unfortunately my structured arrays contain a variety of types outside of |
Indeed, the problem was that prior to 1.12, the structured array formatters simply ignored the numpy formats (and thus deferred to python). |
The cause of issue #5692 is that
_array2string
(in numpy/core/arrayprint.py) doesn’t have format function for structured arrays and it uses general purpose format function to format array elements of structured arrays.I made
StructureFormat
class to format structured array elements._get_format_function
instantiatesStructureFormat
by instantiating format function classes for each field of structure recursively and merge them.