From 6c301804285a18453ba17894f673446631c6f74b Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 26 May 2025 13:55:30 +0100 Subject: [PATCH 1/3] refactor: Simplify `is_ordered_categorical` Part of #2492 --- narwhals/utils.py | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/narwhals/utils.py b/narwhals/utils.py index c3b929906a..58e3472b35 100644 --- a/narwhals/utils.py +++ b/narwhals/utils.py @@ -42,8 +42,6 @@ get_pyspark_connect, get_pyspark_sql, get_sqlframe, - is_cudf_series, - is_modin_series, is_narwhals_series, is_narwhals_series_int, is_numpy_array_1d, @@ -1148,7 +1146,7 @@ def scale_bytes(sz: int, unit: SizeUnit) -> int | float: raise ValueError(msg) -def is_ordered_categorical(series: Series[Any]) -> bool: # noqa: PLR0911 +def is_ordered_categorical(series: Series[Any]) -> bool: """Return whether indices of categories are semantically meaningful. This is a convenience function to accessing what would otherwise be @@ -1194,29 +1192,27 @@ def is_ordered_categorical(series: Series[Any]) -> bool: # noqa: PLR0911 dtypes = series._compliant_series._version.dtypes compliant = series._compliant_series + # If it doesn't match any branches, let's just play it safe and return False. + result: bool = False if isinstance(compliant, InterchangeSeries) and isinstance( series.dtype, dtypes.Categorical ): - return compliant.native.describe_categorical["is_ordered"] - if series.dtype == dtypes.Enum: - return True - if series.dtype != dtypes.Categorical: - return False - native_series = series.to_native() - if is_polars_series(native_series): - return native_series.dtype.ordering == "physical" # type: ignore[attr-defined] - if is_pandas_series(native_series): - return bool(native_series.cat.ordered) - if is_modin_series(native_series): # pragma: no cover - return native_series.cat.ordered - if is_cudf_series(native_series): # pragma: no cover - return native_series.cat.ordered - if is_pyarrow_chunked_array(native_series): - from narwhals._arrow.utils import is_dictionary - - return is_dictionary(native_series.type) and native_series.type.ordered - # If it doesn't match any of the above, let's just play it safe and return False. - return False # pragma: no cover + result = compliant.native.describe_categorical["is_ordered"] + elif series.dtype == dtypes.Enum: + result = True + elif series.dtype != dtypes.Categorical: + result = False + else: + native = series.to_native() + if is_polars_series(native): + result = native.dtype.ordering == "physical" # type: ignore[attr-defined] + elif is_pandas_like_series(native): + result = bool(native.cat.ordered) + elif is_pyarrow_chunked_array(native): + from narwhals._arrow.utils import is_dictionary + + result = is_dictionary(native.type) and native.type.ordered + return result def generate_unique_token( From f957e7fa57201e0c8d83b3065a2494120b88a709 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 26 May 2025 14:00:08 +0100 Subject: [PATCH 2/3] fix(typing): Ensure `pl.Categorical` is in scope --- narwhals/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/narwhals/utils.py b/narwhals/utils.py index 58e3472b35..931beddd62 100644 --- a/narwhals/utils.py +++ b/narwhals/utils.py @@ -60,6 +60,7 @@ from typing import AbstractSet as Set import pandas as pd + import polars as pl import pyarrow as pa from typing_extensions import ( Concatenate, @@ -1205,7 +1206,7 @@ def is_ordered_categorical(series: Series[Any]) -> bool: else: native = series.to_native() if is_polars_series(native): - result = native.dtype.ordering == "physical" # type: ignore[attr-defined] + result = cast("pl.Categorical", native.dtype).ordering == "physical" elif is_pandas_like_series(native): result = bool(native.cat.ordered) elif is_pyarrow_chunked_array(native): From bcde0b9eb6c528cdc29ff5a4e0bb7d7b2ed44b19 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 26 May 2025 14:28:51 +0100 Subject: [PATCH 3/3] test: Coverage for unknown categorical series https://github.com/narwhals-dev/narwhals/actions/runs/15254628546/job/42899191471?pr=2618 --- .../is_ordered_categorical_test.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/series_only/is_ordered_categorical_test.py b/tests/series_only/is_ordered_categorical_test.py index 88149541af..8357f5938c 100644 --- a/tests/series_only/is_ordered_categorical_test.py +++ b/tests/series_only/is_ordered_categorical_test.py @@ -5,12 +5,28 @@ import pytest import narwhals as nw +from narwhals.utils import Version if TYPE_CHECKING: from narwhals.typing import IntoSeries from tests.utils import ConstructorEager +class MockCompliantSeries: + _version = Version.MAIN + + def __narwhals_series__(self) -> Any: + return self + + @property + def native(self) -> tuple[()]: + return () + + @property + def dtype(self) -> nw.Categorical: + return nw.Categorical() + + def test_is_ordered_categorical_polars() -> None: pytest.importorskip("polars") import polars as pl @@ -62,3 +78,8 @@ def test_is_ordered_categorical_pyarrow() -> None: assert nw.is_ordered_categorical( nw.from_native(s, series_only=True) ) # pragma: no cover + + +def test_is_ordered_categorical_unknown_series() -> None: + series: nw.Series[Any] = nw.Series(MockCompliantSeries(), level="full") + assert nw.is_ordered_categorical(series) is False