deprecate positional args

Author: TomAugspurger

src/zarr/_compat.py

@@ -0,0 +1,66 @@
+import warnings
+from collections.abc import Callable
+from functools import wraps
+from inspect import Parameter, signature
+from typing import Any, TypeVar
+
+T = TypeVar("T")
+
+
+def _deprecate_positional_args(
+    func: Callable[..., T] | None = None, *, version: str = "1.3"
+) -> Callable[..., T]:
+    """Decorator for methods that issues warnings for positional arguments.
+
+    Using the keyword-only argument syntax in pep 3102, arguments after the
+    * will issue a warning when passed as a positional argument.
+
+    Parameters
+    ----------
+    func : callable, default=None
+        Function to check arguments on.
+    version : callable, default="1.3"
+        The version when positional arguments will result in error.
+    """
+
+    def _inner_deprecate_positional_args(f: Callable[..., T]) -> Callable[..., T]:
+        sig = signature(f)
+        kwonly_args = []
+        all_args = []
+
+        for name, param in sig.parameters.items():
+            if param.kind == Parameter.POSITIONAL_OR_KEYWORD:
+                all_args.append(name)
+            elif param.kind == Parameter.KEYWORD_ONLY:
+                kwonly_args.append(name)
+
+        @wraps(f)
+        def inner_f(*args: Any, **kwargs: Any) -> T:
+            extra_args = len(args) - len(all_args)
+            if extra_args <= 0:
+                return f(*args, **kwargs)
+
+            # extra_args > 0
+            args_msg = [
+                f"{name}={arg}"
+                for name, arg in zip(kwonly_args[:extra_args], args[-extra_args:], strict=False)
+            ]
+            formatted_args_msg = ", ".join(args_msg)
+            warnings.warn(
+                (
+                    f"Pass {formatted_args_msg} as keyword args. From version "
+                    f"{version} passing these as positional arguments "
+                    "will result in an error"
+                ),
+                FutureWarning,
+                stacklevel=2,
+            )
+            kwargs.update(zip(sig.parameters, args, strict=False))
+            return f(**kwargs)
+
+        return inner_f
+
+    if func is not None:
+        return _inner_deprecate_positional_args(func)
+
+    return _inner_deprecate_positional_args  # type: ignore[return-value]

src/zarr/api/synchronous.py

@@ -3,6 +3,7 @@
 from typing import Any
 
 import zarr.api.asynchronous as async_api
+from zarr._compat import _deprecate_positional_args
 from zarr.core.array import Array, AsyncArray
 from zarr.core.buffer import NDArrayLike
 from zarr.core.common import JSON, AccessModeLiteral, ChunkCoords, ZarrFormat
@@ -61,6 +62,7 @@ def load(
     return sync(async_api.load(store=store, zarr_version=zarr_version, path=path))
 
 
+@_deprecate_positional_args
 def open(
     store: StoreLike | None = None,
     *,
@@ -105,6 +107,7 @@ def save(
     )
 
 
+@_deprecate_positional_args
 def save_array(
     store: StoreLike,
     arr: NDArrayLike,
@@ -155,9 +158,10 @@ def array(data: NDArrayLike, **kwargs: Any) -> Array:
     return Array(sync(async_api.array(data=data, **kwargs)))
 
 
+@_deprecate_positional_args
 def group(
-    *,  # Note: this is a change from v2
     store: StoreLike | None = None,
+    *,  # Note: this is a change from v2
     overwrite: bool = False,
     chunk_store: StoreLike | None = None,  # not used in async_api
     cache_attrs: bool | None = None,  # default changed, not used in async_api
@@ -186,6 +190,7 @@ def group(
     )
 
 
+@_deprecate_positional_args
 def open_group(
     store: StoreLike | None = None,
     *,  # Note: this is a change from v2

src/zarr/core/array.py

@@ -9,6 +9,7 @@
 import numpy as np
 import numpy.typing as npt
 
+from zarr._compat import _deprecate_positional_args
 from zarr.abc.codec import Codec, CodecPipeline
 from zarr.abc.store import set_or_delete
 from zarr.codecs import BytesCodec
@@ -604,6 +605,7 @@ class Array:
     _async_array: AsyncArray
 
     @classmethod
+    @_deprecate_positional_args
     def create(
         cls,
         store: StoreLike,
@@ -999,6 +1001,7 @@ def __setitem__(self, selection: Selection, value: npt.ArrayLike) -> None:
         else:
             self.set_basic_selection(cast(BasicSelection, pure_selection), value, fields=fields)
 
+    @_deprecate_positional_args
     def get_basic_selection(
         self,
         selection: BasicSelection = Ellipsis,
@@ -1122,6 +1125,7 @@ def get_basic_selection(
             )
         )
 
+    @_deprecate_positional_args
     def set_basic_selection(
         self,
         selection: BasicSelection,
@@ -1217,6 +1221,7 @@ def set_basic_selection(
         indexer = BasicIndexer(selection, self.shape, self.metadata.chunk_grid)
         sync(self._async_array._set_selection(indexer, value, fields=fields, prototype=prototype))
 
+    @_deprecate_positional_args
     def get_orthogonal_selection(
         self,
         selection: OrthogonalSelection,
@@ -1341,6 +1346,7 @@ def get_orthogonal_selection(
             )
         )
 
+    @_deprecate_positional_args
     def set_orthogonal_selection(
         self,
         selection: OrthogonalSelection,
@@ -1451,6 +1457,7 @@ def set_orthogonal_selection(
             self._async_array._set_selection(indexer, value, fields=fields, prototype=prototype)
         )
 
+    @_deprecate_positional_args
     def get_mask_selection(
         self,
         mask: MaskSelection,
@@ -1533,6 +1540,7 @@ def get_mask_selection(
             )
         )
 
+    @_deprecate_positional_args
     def set_mask_selection(
         self,
         mask: MaskSelection,
@@ -1611,6 +1619,7 @@ def set_mask_selection(
         indexer = MaskIndexer(mask, self.shape, self.metadata.chunk_grid)
         sync(self._async_array._set_selection(indexer, value, fields=fields, prototype=prototype))
 
+    @_deprecate_positional_args
     def get_coordinate_selection(
         self,
         selection: CoordinateSelection,
@@ -1700,6 +1709,7 @@ def get_coordinate_selection(
             out_array = np.array(out_array).reshape(indexer.sel_shape)
         return out_array
 
+    @_deprecate_positional_args
     def set_coordinate_selection(
         self,
         selection: CoordinateSelection,
@@ -1789,6 +1799,7 @@ def set_coordinate_selection(
 
         sync(self._async_array._set_selection(indexer, value, fields=fields, prototype=prototype))
 
+    @_deprecate_positional_args
     def get_block_selection(
         self,
         selection: BasicSelection,
@@ -1887,6 +1898,7 @@ def get_block_selection(
             )
         )
 
+    @_deprecate_positional_args
     def set_block_selection(
         self,
         selection: BasicSelection,

tests/v3/test_api.py

@@ -1,3 +1,5 @@
+import warnings
+
 import numpy as np
 import pytest
 from numpy.testing import assert_array_equal
@@ -6,7 +8,8 @@
 import zarr
 from zarr import Array, Group
 from zarr.abc.store import Store
-from zarr.api.synchronous import create, load, open, open_group, save, save_array, save_group
+from zarr.api.synchronous import create, group, load, open, open_group, save, save_array, save_group
+from zarr.store.memory import MemoryStore
 
 
 def test_create_array(memory_store: Store) -> None:
@@ -849,3 +852,37 @@ def test_tree() -> None:
 #         # bad option
 #         with pytest.raises(TypeError):
 #             copy(source["foo"], dest, dry_run=True, log=True)
+
+
+def test_open_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    with pytest.warns(FutureWarning, match="pass"):
+        open(store, "w", shape=(1,))
+
+
+def test_save_array_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    with warnings.catch_warnings():
+        warnings.filterwarnings(
+            "ignore", message="zarr_version is deprecated", category=DeprecationWarning
+        )
+        with pytest.warns(FutureWarning, match="pass"):
+            save_array(
+                store,
+                np.ones(
+                    1,
+                ),
+                3,
+            )
+
+
+def test_group_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    with pytest.warns(FutureWarning, match="pass"):
+        group(store, True)
+
+
+def test_open_group_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    with pytest.warns(FutureWarning, match="pass"):
+        open_group(store, "w")

tests/v3/test_array.py

@@ -4,6 +4,7 @@
 import pytest
 
 from zarr import Array, Group
+from zarr.core.buffer import NDBuffer
 from zarr.core.common import ZarrFormat
 from zarr.errors import ContainsArrayError, ContainsGroupError
 from zarr.store import LocalStore, MemoryStore
@@ -135,3 +136,44 @@ def test_array_v3_fill_value(store: MemoryStore, fill_value: int, dtype_str: str
 
     assert arr.fill_value == np.dtype(dtype_str).type(fill_value)
     assert arr.fill_value.dtype == arr.dtype
+
+
+def test_create_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    with pytest.warns(FutureWarning, match="Pass"):
+        Array.create(store, (2, 2), dtype="f8")
+
+
+def test_selection_positional_args_deprecated():
+    store = MemoryStore({}, mode="w")
+    arr = Array.create(store, shape=(2, 2), dtype="f8")
+
+    with pytest.warns(FutureWarning, match="Pass out"):
+        arr.get_basic_selection(..., NDBuffer(array=np.empty((2, 2))))
+
+    with pytest.warns(FutureWarning, match="Pass fields"):
+        arr.set_basic_selection(..., 1, None)
+
+    with pytest.warns(FutureWarning, match="Pass out"):
+        arr.get_orthogonal_selection(..., NDBuffer(array=np.empty((2, 2))))
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.set_orthogonal_selection(..., 1, None)
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.get_mask_selection(np.zeros((2, 2), dtype=bool), NDBuffer(array=np.empty((0,))))
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.set_mask_selection(np.zeros((2, 2), dtype=bool), 1, None)
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.get_coordinate_selection(([0, 1], [0, 1]), NDBuffer(array=np.empty((2,))))
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.set_coordinate_selection(([0, 1], [0, 1]), 1, None)
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.get_block_selection((0, slice(None)), NDBuffer(array=np.empty((2, 2))))
+
+    with pytest.warns(FutureWarning, match="Pass"):
+        arr.set_block_selection((0, slice(None)), 1, None)

tests/v3/test_sync.py

@@ -122,3 +122,6 @@ def bar(self) -> list[int]:
     foo = SyncFoo(async_foo)
     assert foo.foo() == "foo"
     assert foo.bar() == list(range(10))
+
+
+def test_open_positional_args_deprecate(): ...