Deltares / xugrid

Xarray and unstructured grids
https://deltares.github.io/xugrid/
MIT License
58 stars 8 forks source link

Deal with dependencies on xarray core #164

Closed Huite closed 11 months ago

Huite commented 11 months ago

I've released release 0.6.5 with the idea of using less private functionality of xarray. I should've read the FAQ better:

As a rule, only functions/methods documented in our API reference are considered part of xarray’s public API. Everything else (in particular, everything in xarray.core that is not also exposed in the top level xarray namespace) is considered a private implementation detail that may change at any time.

Objects that exist to facilitate xarray’s fluent interface on DataArray and Dataset objects are a special case. For convenience, we document them in the API docs, but only their methods and the DataArray/Dataset methods/properties to construct them (e.g., .plot(), .groupby(), .str) are considered public API. Constructors and other details of the internal classes used to implemented them (i.e., xarray.plot.plotting._PlotMethods, xarray.core.groupby.DataArrayGroupBy, xarray.core.accessor_str.StringAccessor) are not.

(Worth noting is that this is the first time so far that we've had a break due to changes in xarray, so they are relatively stable.)

We should undo the work of last Friday (remove plot_utils) since it doesn't do anything to decrease the dependency surface.

@Hofer-Julian suggests using dependabot.

Huite commented 11 months ago

The non-plotting dependencies are minimal:

either_dict_or_kwargs:

# It's probably OK to give this as a TypeGuard; though it's not perfectly robust.
def is_dict_like(value: Any) -> TypeGuard[Mapping]:
    return hasattr(value, "keys") and hasattr(value, "__getitem__")

def either_dict_or_kwargs(
    pos_kwargs: Mapping[Any, T] | None,
    kw_kwargs: Mapping[str, T],
    func_name: str,
) -> Mapping[Hashable, T]:
    if pos_kwargs is None or pos_kwargs == {}:
        # Need an explicit cast to appease mypy due to invariance; see
        # https://github.com/python/mypy/issues/6228
        return cast(Mapping[Hashable, T], kw_kwargs)

    if not is_dict_like(pos_kwargs):
        raise ValueError(f"the first argument to .{func_name} must be a dictionary")
    if kw_kwargs:
        raise ValueError(
            f"cannot specify both keyword and positional arguments to .{func_name}"
        )
    return pos_kwargs

UncachedAccessor, also used for plotting:


_Accessor = TypeVar("_Accessor")

class UncachedAccessor(Generic[_Accessor]):
    """Acts like a property, but on both classes and class instances

    This class is necessary because some tools (e.g. pydoc and sphinx)
    inspect classes for which property returns itself and not the
    accessor.
    """

    def __init__(self, accessor: type[_Accessor]) -> None:
        self._accessor = accessor

    @overload
    def __get__(self, obj: None, cls) -> type[_Accessor]:
        ...

    @overload
    def __get__(self, obj: object, cls) -> _Accessor:
        ...

    def __get__(self, obj: None | object, cls) -> type[_Accessor] | _Accessor:
        if obj is None:
            return self._accessor

        return self._accessor(obj)  # type: ignore  # assume it is a valid accessor!

It makes sense to duplicate these to xugrid.core.utils or something.

Huite commented 11 months ago

After a bit of research and careful copying and minor editing, all private imports could be easily avoided: https://github.com/Deltares/xugrid/commit/34f61447766104a24cb3292229464d9c1517f8d2