Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BUG: Fix structured array format functions #8200

Merged
merged 3 commits into from
Nov 2, 2016
Merged

BUG: Fix structured array format functions #8200

merged 3 commits into from
Nov 2, 2016

Conversation

skwbc
Copy link
Contributor

@skwbc skwbc commented Oct 22, 2016

1st commit

2nd commit

Closes #8172

Reason of a test failure

test_format_on_flex_array_element (test_regression.TestRegression) fails because of the following TypeError.

In [1]: import numpy as np
   ...: 
   ...: dt = np.dtype([('date', '<M8[D]'), ('val', '<f8')])
   ...: arr = np.array([('2000-01-01', 1)], dt)
   ...: arr[0]

TypeError: input must have type NumPy datetime

It is because array2string calls .item() method of arr[0] and .item() casts the type of date field from numpy.datetime64 to datetime.time.
Then DatetimeFormat calls datetime_as_string with the datetime.time object and TypeError raised (See followings).

In [11]: x = arr[0].item(); x
Out[11]: (datetime.date(2000, 1, 1), 1.0)

In [12]: type(x[0])
Out[12]: datetime.date

In [13]: np.datetime_as_string(x[0])
TypeError: input must have type NumPy datetime

In [14]: type(arr[0][0])
Out[14]: numpy.datetime64

In [15]: np.datetime_as_string(arr[0][0])
Out[15]: '2000-01-01'

I don't understand the mechanism of this casting yet.
If this is a bug we should fix it.
If this is not a bug, I have to fix the code to treat this error in this PR.

@skwbc
BUG: Fix subarray format changed in #8160
2a4dd99 
Preserving structured array element format,
this commit fixes subarray format changed in PR #8160.
This commit also changes iterator for field name from dtype_.descr to
dtype_.names (Related to #8174).
@skwbc skwbc changed the title BUG: Fix structured array format functions WIP: BUG: Fix structured array format functions Oct 22, 2016
@charris charris added this to the 1.12.0 release milestone Oct 22, 2016
@charris
Copy link
Member

charris commented Oct 22, 2016

@ahaldane Just a heads up.

@charris
Copy link
Member

charris commented Oct 28, 2016

We need to get this finished.

@skwbc
BUG: Fix array2string for structured array scalars
a6c2184 
PR #8160 added format function for structured arrays.
But it is not applied for structured array scalars.
Closes #8172
@skwbc
Copy link
Contributor Author

skwbc commented Oct 29, 2016

Reading the document of numpy.ndarray.item, I noticed that x.item() should return standard Python scalar.
So, I fixed array2string function and now all tests have passed.

@charris
Copy link
Member

charris commented Oct 29, 2016

Great! @ahaldane Heads up.

@skwbc skwbc changed the title WIP: BUG: Fix structured array format functions BUG: Fix structured array format functions Oct 29, 2016
@ahaldane
Copy link
Member

I'll take a look soon (today or next few days).

ahaldane
ahaldane reviewed Nov 1, 2016
View reviewed changes
numpy/core/arrayprint.py
is_array_field = 1 < field_values.ndim
format_function = _get_format_function(
ravel(field_values), precision, suppress_small, formatter)
if is_array_field:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if dtype_[field_name].shape != ():, so we don't need is_array_field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it.

numpy/core/arrayprint.py
format_function = _get_format_function(
ravel(field_values), precision, suppress_small, formatter)
if is_array_field:
format_function = SubArrayFormat(format_function)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note: SubarrayFormat here does not take into account precision, suppress_small, formatter. Maybe it should, but if so let's leave that for a future PR. Current numpy master doesn't take them into account either.

numpy/core/arrayprint.py
x = _convert_arrays(x)
lst = style(x)
if a.dtype.fields is not None:
arr = asarray([x], dtype=a.dtype)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be simplified to arr = array([x])?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I fixed it.

numpy/core/arrayprint.py
def __call__(self, arr):
if arr.ndim <= 1:
return "[" + ", ".join(self.format_function(a) for a in arr) + "]"
return "[" + ", ".join(self.__call__(a) for a in arr) + "]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just just to note, probably a more exhaustive solution would recursively call array2string somehow (but with very large max_line_width?). But this looks fine for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so too. First, I tried it a little and found it's not easy. So, I made SubArrayFormat class as the second way.

numpy/core/arrayprint.py
if a.shape == ():
x = a.item()
if isinstance(x, tuple):
x = _convert_arrays(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the two places _convert_arrays is called, which is the old-style structure formatter. Is the other call (in _formatArray) still needed? It looks like that one can never be reached. If so, you might remove _convert_arrays completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all _convert_arrays and replace the code on rank == 0 with ValueError to prevent misuse of _formatArray.

@ahaldane
Copy link
Member

ahaldane commented Nov 2, 2016

All right, LGTM.

Thanks a lot for the followup @skwbc! Merging now.

@ahaldane ahaldane merged commit 94b36b5 into numpy:master Nov 2, 2016
@homu homu mentioned this pull request Nov 2, 2016
Closed
This was referenced Nov 2, 2016
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahaldane ahaldane ahaldane left review comments

Assignees
No one assigned
Labels
00 - Bug component: numpy._core
Projects
None yet
Milestone
1.12.0 release
Development

Successfully merging this pull request may close these issues.

Changed printing of structured arrays does not extend to array scalars
3 participants
@skwbc @charris @ahaldane