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.

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

Update contains_cftime_datetimes to avoid loading entire variable array #7494

Merged
merged 23 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ Bug fixes
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_ and `Scott Chamberlin <https://github.com/scottcha>`_.
- Handle ``keep_attrs`` option in binary operators of :py:meth:`Dataset` (:issue:`7390`, :pull:`7391`).
By `Aron Gergely <https://github.com/arongergely>`_.
- Improved performance in ``open_dataset`` for datasets with large object arrays (:issue:`7484`, :pull:`7494`).
dcherian marked this conversation as resolved.
Show resolved Hide resolved
By `Alex Goodman <https://github.com/agoodm>`_.

Documentation
~~~~~~~~~~~~~
Expand Down
2 changes: 1 addition & 1 deletion xarray/core/accessor_dt.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ def __new__(cls, obj: T_DataArray) -> CombinedDatetimelikeAccessor:
# we need to choose which parent (datetime or timedelta) is
# appropriate. Since we're checking the dtypes anyway, we'll just
# do all the validation here.
if not _contains_datetime_like_objects(obj):
if not _contains_datetime_like_objects(obj.variable):
raise TypeError(
"'.dt' accessor only available for "
"DataArray with datetime64 timedelta64 dtype or "
Expand Down
41 changes: 24 additions & 17 deletions xarray/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
ScalarOrArray,
SideOptions,
T_DataWithCoords,
T_Variable,
)
from xarray.core.variable import Variable

Expand Down Expand Up @@ -1771,31 +1772,37 @@ def is_np_timedelta_like(dtype: DTypeLike) -> bool:
return np.issubdtype(dtype, np.timedelta64)


def _contains_cftime_datetimes(array) -> bool:
def _contains_cftime_datetimes(array: Any) -> bool:
"""Check if an array contains cftime.datetime objects"""
if cftime is None:
return False
from xarray.core.variable import Variable

if isinstance(array, Variable):
var = array
else:
if array.dtype == np.dtype("O") and array.size > 0:
sample = np.asarray(array).flat[0]
if is_duck_dask_array(sample):
sample = sample.compute()
if isinstance(sample, np.ndarray):
sample = sample.item()
return isinstance(sample, cftime.datetime)
else:
return False
var = Variable(dims=tuple(f"dim_{v}" for v in range(array.ndim)), data=array)

return _variable_contains_cftime_datetimes(var)


def contains_cftime_datetimes(var) -> bool:
def _variable_contains_cftime_datetimes(var: T_Variable) -> bool:
"""Check if an xarray.Variable contains cftime.datetime objects"""
if var.dtype == np.dtype("O") and var.size > 0:
return _contains_cftime_datetimes(var.data)
else:
if cftime is None:
return False

if var.dtype == np.dtype("O") and var.size > 0:
first_idx = tuple(0 for _ in range(var.ndim))
Illviljan marked this conversation as resolved.
Show resolved Hide resolved
sample = var[first_idx]
return isinstance(sample.to_numpy().item(), cftime.datetime)

return False


def contains_cftime_datetimes(var: T_Variable) -> bool:
"""Check if an xarray.Variable contains cftime.datetime objects"""
return _variable_contains_cftime_datetimes(var)
agoodm marked this conversation as resolved.
Show resolved Hide resolved


def _contains_datetime_like_objects(var) -> bool:
def _contains_datetime_like_objects(var: T_Variable) -> bool:
"""Check if a variable contains datetime like objects (either
np.datetime64, np.timedelta64, or cftime.datetime)
"""
Expand Down