issue_comments

nice work! shipping...

@max-sixty I just pushed a commit with that change... let's verify that it works in CI and then ship it!

It's not pretty, but this seems to work, on top of the most recent commit 2191fbc:

```diff diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 232dbec3..3b490dcc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,6 +43,7 @@ repos: types-pytz, # Dependencies that are typed numpy, + typing-extensions==3.10.0.0, ] # run this occasionally, ref discussion https://github.com/pydata/xarray/pull/3194 # - repo: https://github.com/asottile/pyupgrade diff --git a/xarray/core/utils.py b/xarray/core/utils.py index 1f2dfb5c..a139d2ef 100644 --- a/xarray/core/utils.py +++ b/xarray/core/utils.py @@ -10,6 +10,7 @@ import warnings from enum import Enum from typing import ( + TYPE_CHECKING, Any, Callable, Collection, @@ -32,7 +33,6 @@ import numpy as np import pandas as pd

- K = TypeVar("K") V = TypeVar("V") T = TypeVar("T") @@ -307,19 +307,29 @@ def _is_scalar(value, include_0d): )

+# See GH5624, this is a convoluted way to allow type-checking to use TypeGuard without +# requiring typing_extensions as a required dependency to run the code (it is required +# to type-check). try: if sys.version_info >= (3, 10): from typing import TypeGuard else: from typing_extensions import TypeGuard except ImportError: - def is_scalar(value: Any, include_0d: bool = True) -> bool: - """Whether to treat a value as a scalar. + if TYPE_CHECKING: + raise + else: + + def is_scalar(value: Any, include_0d: bool = True) -> bool: + """Whether to treat a value as a scalar. + + Any non-iterable, string, or 0-D array + """ + return _is_scalar(value, include_0d) +

• Any non-iterable, string, or 0-D array
• """
• return _is_scalar(value, include_0d) else: + def is_scalar(value: Any, include_0d: bool = True) -> TypeGuard[Hashable]: """Whether to treat a value as a scalar.

```

It works with or without the edit to .pre-commit-config.yaml

@shoyer want me to add it on and merge?

I think requiring typing-extensions for type checks to pass (especially internal to Xarray) would be totally reasonable. In general, it's not a big deal to install a dependency like this with pip/conda but every new dependency can be a marginal annoyance for those who maintain a custom build system of some sort.

To be honest, this only came up because I asked a co-worker to try using the development version of Xarray. Then they discovered that Apache-Beam currently puts on upper bound on the supported typing-extensions version (<= 3.7), so it would not be possible to install the latest versions of both Beam and Xarray :(. Beam should definitely relax this requirement, too, but it seemed like we do could the same, too.

Sorry to be late to the party here — I've been less in the flow of xarray for the past week.

How strongly do you feel about not having this as a dependency @shoyer ? I hadn't thought of it as controversial given it's a "official" python package, but without much confidence so happy to defer.

A few options: - Revert TypeGuard (when would we add it back — when typing-extensions has more track record? When 3.10 is our minimum version?) - Have typing-extensions be optional and add # type: ignore comments. Not sure how many would be required though. - Have typing-extensions be optional but raise an error on if TYPE_CHECKING if it's not installed — i.e. type checking will fail without it, but running xarray would be fine. - Make it a required dependency

If we can get a good error message, I would vote for requiring it for type-checking. If it silently swallows the error and doesn't type check, then I'd probably vote for reverting TypeGuard or making typing-extensions required.