From 74a03a8692f77a8dfa1f507ee44071d8cc3fb611 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 19:30:36 +0100 Subject: [PATCH 1/9] fix(typing): Make `IntoSeries` non-recursive Part of (#2343) --- narwhals/typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/narwhals/typing.py b/narwhals/typing.py index c2c9475409..80bc290091 100644 --- a/narwhals/typing.py +++ b/narwhals/typing.py @@ -102,7 +102,7 @@ def __native_namespace__(self) -> ModuleType: ... ... return df.columns """ -IntoSeries: TypeAlias = Union["Series[Any]", "NativeSeries"] +IntoSeries: TypeAlias = "NativeSeries" """Anything which can be converted to a Narwhals Series. Use this if your function can accept an object which can be converted to `nw.Series` From ff841d1cef7d38a8c1dd64c4c9f6e475f2689a25 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 19:31:38 +0100 Subject: [PATCH 2/9] test: Add `test_series_recursive` Still need to fix `from_native` --- tests/translate/from_native_test.py | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/translate/from_native_test.py b/tests/translate/from_native_test.py index 1ef2805a31..4703b4fb99 100644 --- a/tests/translate/from_native_test.py +++ b/tests/translate/from_native_test.py @@ -367,3 +367,33 @@ def test_from_native_lazyframe() -> None: assert isinstance(stable_lazy, nw.LazyFrame) assert isinstance(unstable_lazy, unstable_nw.LazyFrame) + + +def test_series_recursive() -> None: + """https://github.com/narwhals-dev/narwhals/issues/2239.""" + pytest.importorskip("polars") + import polars as pl + + pl_series = pl.Series(name="test", values=[1, 2, 3]) + nw_series = unstable_nw.from_native(pl_series, series_only=True) + with pytest.raises(AssertionError): + nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") + + nw_series_passthrough = unstable_nw.from_native(nw_series, series_only=True) + + if TYPE_CHECKING: + from typing_extensions import assert_type + + assert_type(pl_series, pl.Series) + assert_type(nw_series, unstable_nw.Series[pl.Series]) + + nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") + # NOTE: Checking that the type is `Series[Unknown]` + assert_type(nw_series_depth_2, unstable_nw.Series) + + # TODO @dangotbanned: Fix this one + # Current: + # `unstable_nw.Series[unstable_nw.Series[pl.Series]]`` + # Goal: + # `unstable_nw.Series[pl.Series]` + assert_type(nw_series_passthrough, unstable_nw.Series[pl.Series]) From 37549cc28bd76e4890301da26149cbff728f7cb8 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:03:38 +0100 Subject: [PATCH 3/9] chore(DRAFT): Add todo --- narwhals/typing.py | 1 + 1 file changed, 1 insertion(+) diff --git a/narwhals/typing.py b/narwhals/typing.py index 80bc290091..e26f92ca67 100644 --- a/narwhals/typing.py +++ b/narwhals/typing.py @@ -37,6 +37,7 @@ def join(self, *args: Any, **kwargs: Any) -> Any: ... class NativeLazyFrame(NativeFrame, Protocol): def explain(self, *args: Any, **kwargs: Any) -> Any: ... + # TODO @dangotbanned: `nw.Series` **cannot** be allowed to match this!!! class NativeSeries(Sized, Iterable[Any], Protocol): def filter(self, *args: Any, **kwargs: Any) -> Any: ... From 6bd47b49ed8259126d286964b6a0110c96e340e8 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:41:57 +0100 Subject: [PATCH 4/9] fix(typing): Try a more aggressive fix --- narwhals/functions.py | 9 +++++---- narwhals/translate.py | 8 +++++++- narwhals/typing.py | 2 ++ tests/translate/from_native_test.py | 6 +++--- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/narwhals/functions.py b/narwhals/functions.py index 56457d3f31..982204f49d 100644 --- a/narwhals/functions.py +++ b/narwhals/functions.py @@ -63,6 +63,7 @@ from narwhals.typing import IntoSeriesT from narwhals.typing import NativeFrame from narwhals.typing import NativeLazyFrame + from narwhals.typing import NativeSeries from narwhals.typing import _2DArray _IntoSchema: TypeAlias = "Mapping[str, DType] | Schema | Sequence[str] | None" @@ -265,7 +266,7 @@ def _new_series_impl( else: # pragma: no cover native_namespace = implementation.to_native_namespace() try: - native_series = native_namespace.new_series(name, values, dtype) + native_series: NativeSeries = native_namespace.new_series(name, values, dtype) return from_native(native_series, series_only=True).alias(name) except AttributeError as e: msg = "Unknown namespace is expected to implement `new_series` constructor." @@ -350,7 +351,7 @@ def _from_dict_impl( try: # implementation is UNKNOWN, Narwhals extension using this feature should # implement `from_dict` function in the top-level namespace. - native_frame = native_namespace.from_dict(data, schema=schema) + native_frame: NativeFrame = native_namespace.from_dict(data, schema=schema) except AttributeError as e: msg = "Unknown namespace is expected to implement `from_dict` function." raise AttributeError(msg) from e @@ -466,7 +467,7 @@ def _from_numpy_impl( try: # implementation is UNKNOWN, Narwhals extension using this feature should # implement `from_numpy` function in the top-level namespace. - native_frame = native_namespace.from_numpy(data, schema=schema) + native_frame: NativeFrame = native_namespace.from_numpy(data, schema=schema) except AttributeError as e: msg = "Unknown namespace is expected to implement `from_numpy` function." raise AttributeError(msg) from e @@ -554,7 +555,7 @@ def _from_arrow_impl( try: # implementation is UNKNOWN, Narwhals extension using this feature should # implement PyCapsule support - native_frame = native_namespace.DataFrame(data) + native_frame: NativeFrame = native_namespace.DataFrame(data) except AttributeError as e: msg = "Unknown namespace is expected to implement `DataFrame` class which accepts object which supports PyCapsule Interface." raise AttributeError(msg) from e diff --git a/narwhals/translate.py b/narwhals/translate.py index c2106c815d..5a1a6b9a98 100644 --- a/narwhals/translate.py +++ b/narwhals/translate.py @@ -50,6 +50,7 @@ from narwhals.typing import IntoLazyFrameT from narwhals.typing import IntoSeries from narwhals.typing import IntoSeriesT + from narwhals.typing import SeriesT T = TypeVar("T") @@ -128,6 +129,10 @@ def to_native( return narwhals_object +@overload +def from_native(native_object: SeriesT, **kwds: Any) -> SeriesT: ... + + @overload def from_native( native_object: IntoDataFrameT | IntoSeries, @@ -298,7 +303,7 @@ def from_native( ) -> Any: ... -def from_native( +def from_native( # noqa: D417 native_object: IntoLazyFrameT | IntoFrameT | IntoSeriesT | IntoFrame | IntoSeries | T, *, strict: bool | None = None, @@ -306,6 +311,7 @@ def from_native( eager_only: bool = False, series_only: bool = False, allow_series: bool | None = None, + **kwds: Any, # noqa: ARG001 ) -> LazyFrame[IntoLazyFrameT] | DataFrame[IntoFrameT] | Series[IntoSeriesT] | T: """Convert `native_object` to Narwhals Dataframe, Lazyframe, or Series. diff --git a/narwhals/typing.py b/narwhals/typing.py index e26f92ca67..37cc08dc93 100644 --- a/narwhals/typing.py +++ b/narwhals/typing.py @@ -178,6 +178,8 @@ def __native_namespace__(self) -> ModuleType: ... LazyFrameT = TypeVar("LazyFrameT", bound="LazyFrame[Any]") +SeriesT = TypeVar("SeriesT", bound="Series[Any]") + IntoSeriesT = TypeVar("IntoSeriesT", bound="IntoSeries") """TypeVar bound to object convertible to Narwhals Series. diff --git a/tests/translate/from_native_test.py b/tests/translate/from_native_test.py index 4703b4fb99..81bd031a7b 100644 --- a/tests/translate/from_native_test.py +++ b/tests/translate/from_native_test.py @@ -377,7 +377,7 @@ def test_series_recursive() -> None: pl_series = pl.Series(name="test", values=[1, 2, 3]) nw_series = unstable_nw.from_native(pl_series, series_only=True) with pytest.raises(AssertionError): - nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") + unstable_nw.Series(nw_series, level="full") nw_series_passthrough = unstable_nw.from_native(nw_series, series_only=True) @@ -387,9 +387,9 @@ def test_series_recursive() -> None: assert_type(pl_series, pl.Series) assert_type(nw_series, unstable_nw.Series[pl.Series]) - nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") + nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") # type: ignore[var-annotated] # NOTE: Checking that the type is `Series[Unknown]` - assert_type(nw_series_depth_2, unstable_nw.Series) + assert_type(nw_series_depth_2, unstable_nw.Series) # type: ignore[type-arg] # TODO @dangotbanned: Fix this one # Current: From 50e4d13d4646840e0e26f432000ea3ca55fb14de Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 21:45:02 +0100 Subject: [PATCH 5/9] test: tidy up --- tests/translate/from_native_test.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/translate/from_native_test.py b/tests/translate/from_native_test.py index 81bd031a7b..a39dba76ed 100644 --- a/tests/translate/from_native_test.py +++ b/tests/translate/from_native_test.py @@ -18,6 +18,7 @@ if TYPE_CHECKING: from typing_extensions import Self + from typing_extensions import assert_type from narwhals.utils import Version @@ -360,8 +361,6 @@ def test_from_native_lazyframe() -> None: stable_lazy = nw.from_native(lf_pl) unstable_lazy = unstable_nw.from_native(lf_pl) if TYPE_CHECKING: - from typing_extensions import assert_type - assert_type(stable_lazy, nw.LazyFrame[pl.LazyFrame]) assert_type(unstable_lazy, unstable_nw.LazyFrame[pl.LazyFrame]) @@ -379,21 +378,13 @@ def test_series_recursive() -> None: with pytest.raises(AssertionError): unstable_nw.Series(nw_series, level="full") - nw_series_passthrough = unstable_nw.from_native(nw_series, series_only=True) + nw_series_early_return = unstable_nw.from_native(nw_series, series_only=True) if TYPE_CHECKING: - from typing_extensions import assert_type - assert_type(pl_series, pl.Series) assert_type(nw_series, unstable_nw.Series[pl.Series]) nw_series_depth_2 = unstable_nw.Series(nw_series, level="full") # type: ignore[var-annotated] # NOTE: Checking that the type is `Series[Unknown]` assert_type(nw_series_depth_2, unstable_nw.Series) # type: ignore[type-arg] - - # TODO @dangotbanned: Fix this one - # Current: - # `unstable_nw.Series[unstable_nw.Series[pl.Series]]`` - # Goal: - # `unstable_nw.Series[pl.Series]` - assert_type(nw_series_passthrough, unstable_nw.Series[pl.Series]) + assert_type(nw_series_early_return, unstable_nw.Series[pl.Series]) From 253da29bc8e02d71c2af411661639bbe67634b93 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 21:46:06 +0100 Subject: [PATCH 6/9] fix(typing): `v1` backport --- narwhals/stable/v1/__init__.py | 8 +++++++- tests/translate/from_native_test.py | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/narwhals/stable/v1/__init__.py b/narwhals/stable/v1/__init__.py index deb33e6481..6779c08ca0 100644 --- a/narwhals/stable/v1/__init__.py +++ b/narwhals/stable/v1/__init__.py @@ -99,6 +99,7 @@ from narwhals.typing import _1DArray from narwhals.typing import _2DArray + SeriesT = TypeVar("SeriesT", bound="Series[Any]") IntoSeriesT = TypeVar("IntoSeriesT", bound="IntoSeries", default=Any) T = TypeVar("T", default=Any) else: @@ -1153,6 +1154,10 @@ def _stableify( return obj +@overload +def from_native(native_object: SeriesT, **kwds: Any) -> SeriesT: ... + + @overload def from_native( native_object: IntoDataFrameT | IntoSeriesT, @@ -1539,7 +1544,7 @@ def from_native( ) -> Any: ... -def from_native( +def from_native( # noqa: D417 native_object: IntoFrameT | IntoFrame | IntoSeriesT | IntoSeries | T, *, strict: bool | None = None, @@ -1548,6 +1553,7 @@ def from_native( eager_or_interchange_only: bool = False, series_only: bool = False, allow_series: bool | None = None, + **kwds: Any, # noqa: ARG001 ) -> LazyFrame[IntoFrameT] | DataFrame[IntoFrameT] | Series[IntoSeriesT] | T: """Convert `native_object` to Narwhals Dataframe, Lazyframe, or Series. diff --git a/tests/translate/from_native_test.py b/tests/translate/from_native_test.py index a39dba76ed..33361760ca 100644 --- a/tests/translate/from_native_test.py +++ b/tests/translate/from_native_test.py @@ -388,3 +388,25 @@ def test_series_recursive() -> None: # NOTE: Checking that the type is `Series[Unknown]` assert_type(nw_series_depth_2, unstable_nw.Series) # type: ignore[type-arg] assert_type(nw_series_early_return, unstable_nw.Series[pl.Series]) + + +def test_series_recursive_v1() -> None: + """https://github.com/narwhals-dev/narwhals/issues/2239.""" + pytest.importorskip("polars") + import polars as pl + + pl_series = pl.Series(name="test", values=[1, 2, 3]) + nw_series = nw.from_native(pl_series, series_only=True) + with pytest.raises(AssertionError): + nw.Series(nw_series, level="full") + + nw_series_early_return = nw.from_native(nw_series, series_only=True) + + if TYPE_CHECKING: + assert_type(pl_series, pl.Series) + assert_type(nw_series, nw.Series[pl.Series]) + + nw_series_depth_2 = nw.Series(nw_series, level="full") + # NOTE: `Unknown` isn't possible for `v1`, as it has a `TypeVar` default + assert_type(nw_series_depth_2, nw.Series[Any]) + assert_type(nw_series_early_return, nw.Series[pl.Series]) From 51aaa8d665dd1318245c88eb9a6f256a9e98944d Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 4 Apr 2025 22:03:53 +0100 Subject: [PATCH 7/9] remove todo --- narwhals/typing.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/narwhals/typing.py b/narwhals/typing.py index 37cc08dc93..6f481768a5 100644 --- a/narwhals/typing.py +++ b/narwhals/typing.py @@ -37,7 +37,6 @@ def join(self, *args: Any, **kwargs: Any) -> Any: ... class NativeLazyFrame(NativeFrame, Protocol): def explain(self, *args: Any, **kwargs: Any) -> Any: ... - # TODO @dangotbanned: `nw.Series` **cannot** be allowed to match this!!! class NativeSeries(Sized, Iterable[Any], Protocol): def filter(self, *args: Any, **kwargs: Any) -> Any: ... @@ -177,7 +176,6 @@ def __native_namespace__(self) -> ModuleType: ... """ LazyFrameT = TypeVar("LazyFrameT", bound="LazyFrame[Any]") - SeriesT = TypeVar("SeriesT", bound="Series[Any]") IntoSeriesT = TypeVar("IntoSeriesT", bound="IntoSeries") From 124239a54ec2460ef23ecbc817d6fcaa5e85e843 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 5 Apr 2025 11:37:41 +0100 Subject: [PATCH 8/9] fix: Add runtime error for unknown `**kwds` https://github.com/narwhals-dev/narwhals/pull/2347#discussion_r2029803888 --- narwhals/stable/v1/__init__.py | 5 ++++- narwhals/translate.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/narwhals/stable/v1/__init__.py b/narwhals/stable/v1/__init__.py index 77212eb90a..fa4f937ca9 100644 --- a/narwhals/stable/v1/__init__.py +++ b/narwhals/stable/v1/__init__.py @@ -1554,7 +1554,7 @@ def from_native( # noqa: D417 eager_or_interchange_only: bool = False, series_only: bool = False, allow_series: bool | None = None, - **kwds: Any, # noqa: ARG001 + **kwds: Any, ) -> LazyFrame[IntoFrameT] | DataFrame[IntoFrameT] | Series[IntoSeriesT] | T: """Convert `native_object` to Narwhals Dataframe, Lazyframe, or Series. @@ -1614,6 +1614,9 @@ def from_native( # noqa: D417 pass_through = validate_strict_and_pass_though( strict, pass_through, pass_through_default=False, emit_deprecation_warning=False ) + if kwds: + msg = f"from_native() got an unexpected keyword argument {next(iter(kwds))!r}" + raise TypeError(msg) result = _from_native_impl( native_object, diff --git a/narwhals/translate.py b/narwhals/translate.py index 5a1a6b9a98..4c7ca1193e 100644 --- a/narwhals/translate.py +++ b/narwhals/translate.py @@ -311,7 +311,7 @@ def from_native( # noqa: D417 eager_only: bool = False, series_only: bool = False, allow_series: bool | None = None, - **kwds: Any, # noqa: ARG001 + **kwds: Any, ) -> LazyFrame[IntoLazyFrameT] | DataFrame[IntoFrameT] | Series[IntoSeriesT] | T: """Convert `native_object` to Narwhals Dataframe, Lazyframe, or Series. @@ -357,6 +357,9 @@ def from_native( # noqa: D417 pass_through = validate_strict_and_pass_though( strict, pass_through, pass_through_default=False, emit_deprecation_warning=True ) + if kwds: + msg = f"from_native() got an unexpected keyword argument {next(iter(kwds))!r}" + raise TypeError(msg) return _from_native_impl( # type: ignore[no-any-return] native_object, From 8f6d90aebf7bee5d6ba0dfa55c525a2cef54bb97 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 5 Apr 2025 11:48:51 +0100 Subject: [PATCH 9/9] test: Add `test_from_native_invalid_keywords` --- tests/translate/from_native_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/translate/from_native_test.py b/tests/translate/from_native_test.py index 33361760ca..9a67c10bc1 100644 --- a/tests/translate/from_native_test.py +++ b/tests/translate/from_native_test.py @@ -5,6 +5,7 @@ from importlib.util import find_spec from typing import TYPE_CHECKING from typing import Any +from typing import Callable from typing import Iterable from typing import Literal from typing import cast @@ -410,3 +411,14 @@ def test_series_recursive_v1() -> None: # NOTE: `Unknown` isn't possible for `v1`, as it has a `TypeVar` default assert_type(nw_series_depth_2, nw.Series[Any]) assert_type(nw_series_early_return, nw.Series[pl.Series]) + + +@pytest.mark.parametrize("from_native", [unstable_nw.from_native, nw.from_native]) +def test_from_native_invalid_keywords(from_native: Callable[..., Any]) -> None: + pattern = r"from_native.+unexpected.+keyword.+bad_1" + + with pytest.raises(TypeError, match=pattern): + from_native(data, bad_1="invalid") + + with pytest.raises(TypeError, match=pattern): + from_native(data, bad_1="invalid", bad_2="also invalid")