From bdd8fe9c43b9ccb4e8c6349f01c37911862ed581 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 18 Jun 2024 13:27:44 +0200 Subject: [PATCH 1/3] fill in shapelike tests, handle negative values correctly, allow 0 --- src/zarr/common.py | 20 ++++++----- tests/v3/test_common.py | 74 +++++++++++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/src/zarr/common.py b/src/zarr/common.py index 9349f9f018..32e4896888 100644 --- a/src/zarr/common.py +++ b/src/zarr/common.py @@ -137,17 +137,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 diff --git a/tests/v3/test_common.py b/tests/v3/test_common.py index cc33aa75cf..7b40e850ec 100644 --- a/tests/v3/test_common.py +++ b/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))]) +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: ... From 32ba006334eeb7a746978d65af18d222f0ff9d21 Mon Sep 17 00:00:00 2001 From: Davis Bennett Date: Wed, 19 Jun 2024 10:29:59 +0200 Subject: [PATCH 2/3] Update tests/v3/test_common.py Co-authored-by: Joe Hamman --- tests/v3/test_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/v3/test_common.py b/tests/v3/test_common.py index 7b40e850ec..cc34208731 100644 --- a/tests/v3/test_common.py +++ b/tests/v3/test_common.py @@ -92,7 +92,7 @@ def test_parse_shapelike_invalid_iterable_types(data: Any) -> None: parse_shapelike(data) -@pytest.mark.parametrize("data", [(1, 2, 3, -1), (-10,)]) +@pytest.mark.parametrize("data", [(1, 2, 3, -1), (-10,), (4.0, 2)]) 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 From e866f247877c5cea462709e3d8fa1f8741ef59e0 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 19 Jun 2024 11:07:49 +0200 Subject: [PATCH 3/3] move float test case to the appropriate test --- tests/v3/test_common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/v3/test_common.py b/tests/v3/test_common.py index cc34208731..bb59789843 100644 --- a/tests/v3/test_common.py +++ b/tests/v3/test_common.py @@ -82,7 +82,7 @@ def test_parse_shapelike_invalid_single_value() -> None: parse_shapelike(-1) -@pytest.mark.parametrize("data", ["shape", ("0", 1, 2, 3), {"0": "0"}, ((1, 2), (2, 2))]) +@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 @@ -92,7 +92,7 @@ def test_parse_shapelike_invalid_iterable_types(data: Any) -> None: parse_shapelike(data) -@pytest.mark.parametrize("data", [(1, 2, 3, -1), (-10,), (4.0, 2)]) +@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