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 13 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 @@ -99,6 +99,8 @@ Bug fixes
- :py:func:`xarray.Dataset.to_zarr` now drops variable encodings that have been added by xarray during reading
a dataset. (:issue:`7129`, :pull:`7500`).
By `Hauke Schulz <https://github.com/observingClouds>`_.
- 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 @@ -574,7 +574,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
34 changes: 17 additions & 17 deletions xarray/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import pandas as pd

from xarray.core import dtypes, duck_array_ops, formatting, formatting_html, ops
from xarray.core.indexing import BasicIndexer, ExplicitlyIndexed
from xarray.core.options import OPTIONS, _get_keep_attrs
from xarray.core.pycompat import is_duck_dask_array
from xarray.core.utils import Frozen, either_dict_or_kwargs, is_scalar
Expand Down Expand Up @@ -40,6 +41,7 @@
ScalarOrArray,
SideOptions,
T_DataWithCoords,
T_Variable,
)
from xarray.core.variable import Variable

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


def _contains_cftime_datetimes(array) -> bool:
"""Check if an array contains cftime.datetime objects"""
def _contains_cftime_datetimes(array: Any) -> bool:
"""Check if a array inside a Variable contains cftime.datetime objects"""
if cftime is None:
return False
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

if array.dtype == np.dtype("O") and array.size > 0:
first_idx = (0,) * array.ndim
if isinstance(array, ExplicitlyIndexed):
first_idx = BasicIndexer(first_idx)
sample = array[first_idx]
return isinstance(np.asarray(sample).item(), cftime.datetime)

return False

def contains_cftime_datetimes(var) -> bool:

def 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:
dcherian marked this conversation as resolved.
Show resolved Hide resolved
return False
return _contains_cftime_datetimes(var._data)


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
11 changes: 11 additions & 0 deletions xarray/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,17 @@ def __getitem__(self, key):
raise UnexpectedDataAccess("Tried accessing data")


class FirstElementAccessibleArray(utils.NDArrayMixin, ExplicitlyIndexed):
def __init__(self, array):
self.array = array

def __getitem__(self, key):
tuple_idxr = key.tuple
if len(tuple_idxr) > 1:
raise UnexpectedDataAccess("Tried accessing more than one element.")
return self.array[tuple_idxr]


class ReturnItem:
def __getitem__(self, key):
return key
Expand Down
16 changes: 15 additions & 1 deletion xarray/tests/test_accessor_dt.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import xarray as xr
from xarray.tests import (
FirstElementAccessibleArray,
assert_array_equal,
assert_chunks_equal,
assert_equal,
Expand Down Expand Up @@ -418,7 +419,6 @@ def test_calendar_cftime(data) -> None:
assert data.time.dt.calendar == expected


@requires_cftime
def test_calendar_datetime64_2d() -> None:
data = xr.DataArray(np.zeros((4, 5), dtype="datetime64[ns]"), dims=("x", "y"))
assert data.dt.calendar == "proleptic_gregorian"
Expand Down Expand Up @@ -668,3 +668,17 @@ def test_cftime_round_accessor(
result = cftime_rounding_dataarray.dt.round(freq)

assert_identical(result, expected)


@requires_cftime
def test_contains_cftime_lazy() -> None:
import cftime

from xarray.core.common import _contains_cftime_datetimes

times = np.array(
[cftime.DatetimeGregorian(1, 1, 2, 0), cftime.DatetimeGregorian(1, 1, 2, 0)],
dtype=object,
)
array = FirstElementAccessibleArray(times)
assert _contains_cftime_datetimes(array)