parse_shapelike allows 0 (#1979)

* fill in shapelike tests, handle negative values correctly, allow 0

* Update tests/v3/test_common.py

Co-authored-by: Joe Hamman <[email protected]>

* move float test case to the appropriate test

---------

Co-authored-by: Joe Hamman <[email protected]>

Author: d-v-b

src/zarr/common.py

@@ -135,17 +135,21 @@ def parse_named_configuration(
 
 def parse_shapelike(data: int | Iterable[int]) -> tuple[int, ...]:
     if isinstance(data, int):
+        if data < 0:
+            raise ValueError(f"Expected a non-negative integer. Got {data} instead")
         return (data,)
-    if not isinstance(data, Iterable):
-        raise TypeError(f"Expected an iterable. Got {data} instead.")
-    data_tuple = tuple(data)
-    if len(data_tuple) == 0:
-        raise ValueError("Expected at least one element. Got 0.")
+    try:
+        data_tuple = tuple(data)
+    except TypeError as e:
+        msg = f"Expected an integer or an iterable of integers. Got {data} instead."
+        raise TypeError(msg) from e
+
     if not all(isinstance(v, int) for v in data_tuple):
-        msg = f"Expected an iterable of integers. Got {type(data)} instead."
+        msg = f"Expected an iterable of integers. Got {data} instead."
         raise TypeError(msg)
-    if not all(lambda v: v > 0 for v in data_tuple):
-        raise ValueError(f"All values must be greater than 0. Got {data}.")
+    if not all(v > -1 for v in data_tuple):
+        msg = f"Expected all values to be non-negative. Got {data} instead."
+        raise ValueError(msg)
     return data_tuple
 
 

tests/v3/test_common.py

@@ -14,28 +14,28 @@
 
 
 @pytest.mark.parametrize("data", [(0, 0, 0, 0), (1, 3, 4, 5, 6), (2, 4)])
-def test_product(data: tuple[int, ...]):
+def test_product(data: tuple[int, ...]) -> None:
     assert product(data) == np.prod(data)
 
 
 # todo: test
-def test_concurrent_map(): ...
+def test_concurrent_map() -> None: ...
 
 
 # todo: test
-def test_to_thread(): ...
+def test_to_thread() -> None: ...
 
 
 # todo: test
-def test_enum_names(): ...
+def test_enum_names() -> None: ...
 
 
 # todo: test
-def test_parse_enum(): ...
+def test_parse_enum() -> None: ...
 
 
 @pytest.mark.parametrize("data", [("foo", "bar"), (10, 11)])
-def test_parse_name_invalid(data: tuple[Any, Any]):
+def test_parse_name_invalid(data: tuple[Any, Any]) -> None:
     observed, expected = data
     if isinstance(observed, str):
         with pytest.raises(ValueError, match=f"Expected '{expected}'. Got {observed} instead."):
@@ -48,47 +48,71 @@ def test_parse_name_invalid(data: tuple[Any, Any]):
 
 
 @pytest.mark.parametrize("data", [("foo", "foo"), ("10", "10")])
-def test_parse_name_valid(data: tuple[Any, Any]):
+def test_parse_name_valid(data: tuple[Any, Any]) -> None:
     observed, expected = data
     assert parse_name(observed, expected) == observed
 
 
 @pytest.mark.parametrize("data", [0, 1, "hello", "f"])
-def test_parse_indexing_order_invalid(data):
+def test_parse_indexing_order_invalid(data: Any) -> None:
     with pytest.raises(ValueError, match="Expected one of"):
         parse_indexing_order(data)
 
 
 @pytest.mark.parametrize("data", ["C", "F"])
-def parse_indexing_order_valid(data: Literal["C", "F"]):
+def parse_indexing_order_valid(data: Literal["C", "F"]) -> None:
     assert parse_indexing_order(data) == data
 
 
-@pytest.mark.parametrize("data", [("0", 1, 2, 3), {"0": "0"}, []])
-def test_parse_shapelike_invalid(data: Any):
-    if isinstance(data, Iterable):
-        if len(data) == 0:
-            with pytest.raises(ValueError, match="Expected at least one element."):
-                parse_shapelike(data)
-        else:
-            with pytest.raises(TypeError, match="Expected an iterable of integers"):
-                parse_shapelike(data)
-    else:
-        with pytest.raises(TypeError, match="Expected an iterable."):
-            parse_shapelike(data)
+@pytest.mark.parametrize("data", [lambda v: v, slice(None)])
+def test_parse_shapelike_invalid_single_type(data: Any) -> None:
+    """
+    Test that we get the expected error message when passing in a value that is not an integer
+    or an iterable of integers.
+    """
+    with pytest.raises(TypeError, match="Expected an integer or an iterable of integers."):
+        parse_shapelike(data)
+
+
+def test_parse_shapelike_invalid_single_value() -> None:
+    """
+    Test that we get the expected error message when passing in a negative integer.
+    """
+    with pytest.raises(ValueError, match="Expected a non-negative integer."):
+        parse_shapelike(-1)
+
+
+@pytest.mark.parametrize("data", ["shape", ("0", 1, 2, 3), {"0": "0"}, ((1, 2), (2, 2)), (4.0, 2)])
+def test_parse_shapelike_invalid_iterable_types(data: Any) -> None:
+    """
+    Test that we get the expected error message when passing in an iterable containing
+    non-integer elements
+    """
+    with pytest.raises(TypeError, match="Expected an iterable of integers"):
+        parse_shapelike(data)
+
+
+@pytest.mark.parametrize("data", [(1, 2, 3, -1), (-10,)])
+def test_parse_shapelike_invalid_iterable_values(data: Any) -> None:
+    """
+    Test that we get the expected error message when passing in an iterable containing negative
+    integers
+    """
+    with pytest.raises(ValueError, match="Expected all values to be non-negative."):
+        parse_shapelike(data)
 
 
-@pytest.mark.parametrize("data", [range(10), [0, 1, 2, 3], (3, 4, 5)])
-def test_parse_shapelike_valid(data: Iterable[Any]):
+@pytest.mark.parametrize("data", [range(10), [0, 1, 2, 3], (3, 4, 5), ()])
+def test_parse_shapelike_valid(data: Iterable[int]) -> None:
     assert parse_shapelike(data) == tuple(data)
 
 
 # todo: more dtypes
 @pytest.mark.parametrize("data", [("uint8", np.uint8), ("float64", np.float64)])
-def parse_dtype(data: tuple[str, np.dtype]):
+def parse_dtype(data: tuple[str, np.dtype]) -> None:
     unparsed, parsed = data
     assert parse_dtype(unparsed) == parsed
 
 
 # todo: figure out what it means to test this
-def test_parse_fill_value(): ...
+def test_parse_fill_value() -> None: ...