Skip to content

Rely on array backend for string formatting#6823

Merged
dcherian merged 11 commits into
pydata:mainfrom
Illviljan:format_data
Aug 8, 2022
Merged

Rely on array backend for string formatting#6823
dcherian merged 11 commits into
pydata:mainfrom
Illviljan:format_data

Conversation

@Illviljan

@Illviljan Illviljan commented Jul 25, 2022

Copy link
Copy Markdown
Contributor
  • Rely on array backend for string formatting instead of forcing to numpy arrays. The previous implementation used .values to force to numpy arrays which doesn't work for sparse arrays and might be scary in the case of large dask arrays.
  • Only allow format_spec when dealing with scalar arrays, shape=(). Using f-strings is a common method to print the repr. If we want to have full support of .__format__ we should probably add format_spec as input to formatting.array_repr.
import pandas as pd
import xarray as xr

s = pd.Series(
    range(4),
    index=pd.MultiIndex.from_product([list("ab"), list("cd")]),
)

da = xr.DataArray.from_series(s, sparse=True)

# Handle sparse:
print(f"Error: {da} is sparse")
Error: <xarray.DataArray (level_0: 2, level_1: 2)>
<COO: shape=(2, 2), dtype=float64, nnz=4, fill_value=nan>
Coordinates:
  * level_0  (level_0) object 'a' 'b'
  * level_1  (level_1) object 'c' 'd' is sparse

# Handle format_spec:
da = xr.DataArray([1, 2, 3])

print(f'{da[0]}')
<xarray.DataArray ()>
array(1)

print(f'{da[0]:d}')
1

print(f'{da[0]:f}')
1.000000

da = xr.DataArray([1, 2, 3])
print(f'{da:.f}')
Traceback (most recent call last):
(...)
NotImplementedError: Using format_spec is only supported when shape is (). Got shape = (3,).

@Illviljan Illviljan changed the title Format data Rely on array backend for string formatting Jul 25, 2022
@Illviljan Illviljan requested a review from fmaussion July 25, 2022 09:50
@Illviljan

Copy link
Copy Markdown
Contributor Author

Hmm, I'm not a fan that f-strings by default uses the backend array only:

import pandas as pd
import xarray as xr

s = pd.Series(
    range(4),
    index=pd.MultiIndex.from_product([list("ab"), list("cd")]),
)

da = xr.DataArray.from_series(s, sparse=True)

# Uses __format__:
print(f"Error: {da} is sparse")
Error: <COO: shape=(2, 2), dtype=float64, nnz=4, fill_value=nan> is sparse

Should it by default just use the repr instead? But it won't feel very consistent once string formatting are used though.

@Illviljan

Copy link
Copy Markdown
Contributor Author

This is how it behaves right now. I believe this should be the same behavior as previously now with the extra possibility to format scalar arrays:

import pandas as pd
import xarray as xr

s = pd.Series(
    range(4),
    index=pd.MultiIndex.from_product([list("ab"), list("cd")]),
)

da = xr.DataArray.from_series(s, sparse=True)

# Handle sparse:
print(f"Error: {da} is sparse")
Error: <xarray.DataArray (level_0: 2, level_1: 2)>
<COO: shape=(2, 2), dtype=float64, nnz=4, fill_value=nan>
Coordinates:
  * level_0  (level_0) object 'a' 'b'
  * level_1  (level_1) object 'c' 'd' is sparse

# Handle format_spec:
da = xr.DataArray([1, 2, 3])

print(f'{da[0]}')
<xarray.DataArray ()>
array(1)

print(f'{da[0]:d}')
1

print(f'{da[0]:f}')
1.000000

da = xr.DataArray([1, 2, 3])
print(f'{da:.f}')
Traceback (most recent call last):
(...)
NotImplementedError: Using format_spec is only supported when shape is (). Got shape = (3,).

@keewis

keewis commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator

But it won't feel very consistent once string formatting are used though.

I guess we could introduce some sort of xarray specific formatting mini-language to solve that? For example:

  • f"{arr:r}": print the object's repr (the default)
  • f"{arr:b}": print the array backend's repr (only for DataArray / Variable)

@Illviljan

Copy link
Copy Markdown
Contributor Author

I guess we could introduce some sort of xarray specific formatting mini-language to solve that? For example:

* `f"{arr:r}": print the object's `repr` (the default)

* `f"{arr:b}": print the array backend's `repr`(only for`DataArray`/`Variable`)

Interesting idea. I was thinking just adding format_spec to formatting.array_repr so the repr can handle __format__ as well should be enough. It seemed much more involved though.

Anyway that's for another PR. I think this PR is ready now, I think it should handle the main request in #5976 without being so intrusive in other cases.

@max-sixty

Copy link
Copy Markdown
Collaborator

Great idea — doing anything that's expensive on repr has caught us a couple of times and can be a frustrating perf issue. Thanks @Illviljan !

@Illviljan Illviljan added the plan to merge Final call for comments label Aug 8, 2022

@keewis keewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good to me, and I agree that adding format strings for Dataset or non-scalar DataArray (if at all) should be done in a new PR

@dcherian dcherian merged commit c3d45b3 into pydata:main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plan to merge Final call for comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RuntimeError when formatting sparse-backed DataArray in f-string

5 participants