BUG: Fix structured array format functions by skwbc · Pull Request #8200 · numpy/numpy

skwbc · 2016-10-22T09:27:03Z

1st commit

Preserving structured array element format, this commit fixes subarray format changed in PR BUG: fix _array2string for structured array (issue #5692) #8160.
This commit also changes iterator for field name from dtype_.descr to dtype_.names.

2nd commit

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.

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).

charris · 2016-10-22T17:08:45Z

@ahaldane Just a heads up.

charris · 2016-10-28T15:49:55Z

We need to get this finished.

PR #8160 added format function for structured arrays. But it is not applied for structured array scalars. Closes #8172

skwbc · 2016-10-29T15:12:57Z

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 · 2016-10-29T15:50:31Z

Great! @ahaldane Heads up.

ahaldane · 2016-10-31T15:36:15Z

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

ahaldane · 2016-11-01T03:09:21Z

+            is_array_field = 1 < field_values.ndim
+            format_function = _get_format_function(
+                    ravel(field_values), precision, suppress_small, formatter)
+            if is_array_field:


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

I fixed it.

ahaldane · 2016-11-01T03:11:28Z

+            format_function = _get_format_function(
+                    ravel(field_values), precision, suppress_small, formatter)
+            if is_array_field:
+                format_function = SubArrayFormat(format_function)


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.

ahaldane · 2016-11-01T03:16:23Z

-            x = _convert_arrays(x)
-        lst = style(x)
+        if a.dtype.fields is not None:
+            arr = asarray([x], dtype=a.dtype)


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

Yes. I fixed it.

ahaldane · 2016-11-01T03:17:38Z

+    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) + "]"


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.

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.

ahaldane · 2016-11-01T03:52:52Z

    if a.shape == ():
        x = a.item()
-        if isinstance(x, tuple):
-            x = _convert_arrays(x)


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.

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

Related to PR #8200

ahaldane · 2016-11-02T02:04:13Z

All right, LGTM.

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

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 changed the title ~~BUG: Fix structured array format functions~~ WIP: BUG: Fix structured array format functions Oct 22, 2016

charris added this to the 1.12.0 release milestone Oct 22, 2016

charris added 00 - Bug component: numpy._core labels Oct 22, 2016

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 changed the title ~~WIP: BUG: Fix structured array format functions~~ BUG: Fix structured array format functions Oct 29, 2016

ahaldane reviewed Nov 1, 2016

View reviewed changes

MAINT: Refactor numpy/core/arrayprint.py

e1326c3

Related to PR #8200

ahaldane merged commit 94b36b5 into numpy:master Nov 2, 2016

homu mentioned this pull request Nov 2, 2016

Fix array2string for specific mat type #7706

Closed

This was referenced Nov 2, 2016

Allow numpy-1.12's formatter for repr of scalar representations. astropy/astropy#5447

Merged

Changed printing of structured arrays does not extend to array scalars #8172

Closed

Conversation

skwbc commented Oct 22, 2016

1st commit

2nd commit

Reason of a test failure

Uh oh!

charris commented Oct 22, 2016

Uh oh!

charris commented Oct 28, 2016

Uh oh!

skwbc commented Oct 29, 2016

Uh oh!

charris commented Oct 29, 2016

Uh oh!

ahaldane commented Oct 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahaldane commented Nov 2, 2016

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants