Make typing-extensions optional

[shoyer]

Type checking may be a little worse if typing-extensions are not installed, but I don't think it's worth the trouble of adding another hard dependency just for one use for TypeGuard.

Note: sadly this doesn't work yet. Mypy (and pylance) don't like the type alias defined with try/except. Any ideas? In the worst case, we could revert the TypeGuard entirely, but that would be a shame...

• Closes Add typing-extensions to the list of dependencies? #5495
• Passes pre-commit run --all-files

[github-actions]

Unit Test Results

         6 files  ±0           6 suites  ±0   56m 9s ⏱️ ±0s
16 204 tests ±0  14 469 ✔️ ±0  1 735 💤 ±0  0 ❌ ±0 
90 420 runs  ±0  82 245 ✔️ ±0  8 175 💤 ±0  0 ❌ ±0 

Results for commit deaca14. ± Comparison against base commit deaca14.

♻️ This comment has been updated with latest results.

[max-sixty]

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.

[shoyer]

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.

[max-sixty]

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

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?

[shoyer]

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

[dcherian]

nice work! shipping...