feat(typing): Backport generic `Series` to `v1` by dangotbanned · Pull Request #2110 · narwhals-dev/narwhals

dangotbanned · 2025-02-27T13:02:13Z

Tracking

What type of PR is this? (check all applicable)

Related issues

Checklist

Code follows style guide (ruff)
Tests added
Documented the changes

If you have comments or can explain your changes, please do so below

Hoping to see if I can reproduce whatever downstream errors were seen in (feat: make Series generic #1412 (comment))
The issue appears to be the same as (fix(typing): Ensure LazyFrame has a unique TypeVar #1930)
- Didn't show up for v1.(Data|Lazy)Frame because they use a different TypeVar to their parent

Related: - #1412 - #1930 - https://narwhals-dev.github.io/narwhals/backcompat/#breaking-changes-carried-out-so-far

That now would trigger **15** `[var-annotated]` warnings in our tests: ```py tests/utils_test.py:39: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pd.Series([1, 2, 3], index=[2, 1, 0]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/utils_test.py:65: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pd.Series([1, 2, 3], index=[2, 2, 0]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/utils_test.py:72: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/utils_test.py:171: error: Need type annotation for "index" [var-annotated] index = nw.from_native(pd.Series([0, 1, 2]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/utils_test.py:198: error: Need type annotation for "series" [var-annotated] series = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/utils_test.py:215: error: Need type annotation for "pandas_series" [var-annotated] pandas_series = nw.from_native( ^ tests/utils_test.py:232: error: Need type annotation for "series" [var-annotated] series = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/stable_api_test.py:136: error: Need type annotation for "stable_df" [var-annotated] stable_df = nw_v1.from_native(pl.Series(), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/dtypes_test.py:152: error: Need type annotation for "result" [var-annotated] result = nw.from_native(s, series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/dtypes_test.py:182: error: Need type annotation for "snw" [var-annotated] snw = nw.from_native(s, series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/translate/from_native_test.py:140: error: Need type annotation for "res" [var-annotated] res = nw.from_native(obj, series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/series_only/hist_test.py:277: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/series_only/arrow_c_stream_test.py:18: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/series_only/arrow_c_stream_test.py:31: error: Need type annotation for "s" [var-annotated] s = nw.from_native(pl.Series([1, 2, 3]), series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tests/expr_and_series/cast_test.py:172: error: Need type annotation for "s" [var-annotated] s = nw.from_native(s_pd, series_only=True) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Found 15 errors in 7 files (checked 345 source files) ```

Hoping to fix: - https://github.com/narwhals-dev/narwhals/actions/runs/13567019217/job/37922302040?pr=2110 - https://github.com/narwhals-dev/narwhals/actions/runs/13567019217/job/37922302040?pr=2110

dangotbanned · 2025-02-27T13:14:07Z

tests/translate/from_native_test.py

    with pytest.raises(TypeError, match="Cannot only use `series_only`"):
-        nw.from_native(df, series_only=True)
+        nw.from_native(df, series_only=True)  # pyright: ignore[reportArgumentType, reportCallIssue]


Needing this ignore makes me think I'm onto something with (daa4a17)

dangotbanned · 2025-02-27T13:16:47Z

I can just fix this on the altair side? https://github.com/narwhals-dev/narwhals/actions/runs/13567213313/job/37922925743?pr=2110

Pretty sure it would be working if I change strict=True -> pass_through=False.
Similar to #2110 (comment)

Update 1

Following (vega/altair#3811), this warning for altair is valid.

But I can't add an ignore in altair until this is merged

altair/utils/schemapi.py:497: error: No overload variant of "from_native"
matches argument types "Iterable[Any]", "bool"  [call-overload]
            ser = nw.from_native(obj, series_only=True)
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
altair/utils/schemapi.py:497: note: Possible overload variants:

https://github.com/vega/altair/blob/c0956c5ba7de45e1a6b67b2051a10f9477c7afe0/altair/utils/schemapi.py#L495-L500

If anything - this is demonstrating the @overload(s) are now working as intended.
The fact I'm catching a TypeError should require the need of a type: ignore:

def _from_array_like(obj: Iterable[Any], /) -> list[Any]:
    try:
        ser = nw.from_native(obj, series_only=True)
        return ser.to_list()
    except TypeError:
        return list(obj)

Update 2

See #2110 (comment)

dangotbanned · 2025-02-27T13:18:15Z

https://github.com/narwhals-dev/narwhals/actions/runs/13567213313/job/37922929104?pr=2110

shiny is only failing because it is running our test suite and failing #2109

Spotted during an upstream integration test - narwhals-dev/narwhals#2110 (comment) IIRC, I added `strict` before it was [deprecated](https://narwhals-dev.github.io/narwhals/backcompat/#breaking-changes-carried-out-so-far) to help match an `@overload`. Since then, the `@overload` definitions in `narwhals` have improved - meaning the default **doesn't** need to be passed in for us to get it typing correctly # Related - #3693 - narwhals-dev/narwhals#2110

#2110 (comment)

tests/translate/from_native_test.py

MarcoGorelli · 2025-02-27T14:54:46Z

Thanks - I haven't looked at the shiny failure closely, but if that and the altair one are easy fixes then I think we can do this

dangotbanned · 2025-02-27T15:09:34Z

Thanks - I haven't looked at the shiny failure closely, but if that and the altair one are easy fixes then I think we can do this

Great thanks @MarcoGorelli - the shiny one should be solved by (#2109) - or at least most of it

dangotbanned · 2025-02-27T15:58:35Z

narwhals/stable/v1/__init__.py

+    IntoSeriesT = TypeVar("IntoSeriesT", bound="IntoSeries", default=Any)
+    T = TypeVar("T", default=Any)
+else:
+    from typing import TypeVar
+
+    IntoSeriesT = TypeVar("IntoSeriesT", bound="IntoSeries")
+    T = TypeVar("T")


Using PEP 696 is intended to limit the impact of the change downstream.

Code that currently uses v1.Series should fall back to the default v1.Series[Any].
Instead of appearing as v1.Series[Unknown].

I'm not sure if that was a problem in #1412, since the logs have expired https://github.com/narwhals-dev/narwhals/actions/runs/11932852152/job/33258672137?pr=1412

dangotbanned · 2025-02-27T19:04:35Z

Thanks - I haven't looked at the shiny failure closely, but if that and the altair one are easy fixes then I think we can do this

Hey @schloerke 👋!

I saw you were active on #1412 (comment) and have recently made a typing PR (posit-dev/py-shiny#1875)

Anyways, I wanted to check in and see if (#2110 (comment)) sounds like a reasonable assessment of this integration failure run

Update

All looking good in (https://github.com/narwhals-dev/narwhals/actions/runs/13575555103/job/37950799431?pr=2110)

Seems like (#2109) was the only issue for shiny

dangotbanned · 2025-02-27T21:01:24Z

@MarcoGorelli I believe we're all good now 😁, just altair reporting a true positive (https://github.com/narwhals-dev/narwhals/actions/runs/13575555103/job/37950799078?pr=2110)

MarcoGorelli

legend mate

Have a better understanding *now* of what the issue is in (https://github.com/narwhals-dev/narwhals/actions/runs/13577097131/job/37955718895). - This patch is *functionally* the same - but removes the need for ignoring a (typing) warning following (narwhals-dev/narwhals@62f8a1f). - Should also mean we don't need to bump our `narwhals` minimum [again](#3807 (comment)). - **Bonus**: reduced overhead for `<3.11` (python/cpython#84403) ## Related - narwhals-dev/narwhals#2110 - #3811 - narwhals-dev/narwhals#2110 (comment) - narwhals-dev/narwhals#2110 (comment) - narwhals-dev/narwhals#2111

schloerke · 2025-03-10T15:40:17Z

Thank you @dangotbanned ! Very exciting to have access to IntoSeriesT 😃

schloerke · 2025-03-10T15:45:59Z

Q: Is Shiny the only package holding back on typing updates for v1 stable?

It's a slower process compared to your fast releases... but in a few weeks, I could take a look to see if Shiny can get typing updates for Narwhals v1 (not stable) and bump the min version for Narwhals.

dangotbanned added 2 commits February 27, 2025 12:54

feat(typing): Backport generic Series to v1

6d5d03e

Related: - #1412 - #1930 - https://narwhals-dev.github.io/narwhals/backcompat/#breaking-changes-carried-out-so-far

dangotbanned added enhancement New feature or request typing labels Feb 27, 2025

fix(typing): Try with a default for downstream

daa4a17

Hoping to fix: - https://github.com/narwhals-dev/narwhals/actions/runs/13567019217/job/37922302040?pr=2110 - https://github.com/narwhals-dev/narwhals/actions/runs/13567019217/job/37922302040?pr=2110

dangotbanned commented Feb 27, 2025

View reviewed changes

dangotbanned mentioned this pull request Feb 27, 2025

chore: Remove deprecated nw.from_native(..., strict=True) vega/altair#3811

Merged

test(typing): add repro for bug found in altair

3ca6dab

#2110 (comment)

dangotbanned commented Feb 27, 2025

View reviewed changes

tests/translate/from_native_test.py Show resolved Hide resolved

dangotbanned marked this pull request as ready for review February 27, 2025 14:53

dangotbanned mentioned this pull request Feb 27, 2025

Tighten-up NativeSeries Protocol #2111

Closed

dangotbanned commented Feb 27, 2025

View reviewed changes

dangotbanned mentioned this pull request Feb 27, 2025

feat: Adds DataFrame.iter_columns #2104

Merged

12 tasks

Merge branch 'main' into v1-series-generic-backport

3eac716

MarcoGorelli approved these changes Feb 27, 2025

View reviewed changes

MarcoGorelli merged commit 62f8a1f into main Feb 27, 2025
27 of 29 checks passed

MarcoGorelli deleted the v1-series-generic-backport branch February 27, 2025 21:55

This was referenced Feb 28, 2025

chore(typing): Follow-up fix _from_array_like vega/altair#3812

Merged

fix(typing): Narrow NativeSeries Protocol #2159

Merged

dangotbanned mentioned this pull request Mar 9, 2025

refactor: adds _compliant sub-package #2149

Merged

10 tasks

dangotbanned mentioned this pull request Mar 13, 2025

refactor: Add CompliantSeries.from_numpy #2196

Merged

10 tasks

dangotbanned mentioned this pull request Nov 21, 2025

fix(typing): Overhaul @overloads in nw.from_native #3125

Merged

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(typing): Backport generic `Series` to `v1`#2110

feat(typing): Backport generic `Series` to `v1`#2110
MarcoGorelli merged 5 commits intomainfrom
v1-series-generic-backport

dangotbanned commented Feb 27, 2025 •

edited

Loading

Uh oh!

dangotbanned Feb 27, 2025

Uh oh!

dangotbanned commented Feb 27, 2025 •

edited

Loading

Uh oh!

dangotbanned commented Feb 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

MarcoGorelli commented Feb 27, 2025

Uh oh!

dangotbanned commented Feb 27, 2025

Uh oh!

dangotbanned Feb 27, 2025

Uh oh!

dangotbanned commented Feb 27, 2025 •

edited

Loading

Uh oh!

dangotbanned commented Feb 27, 2025

Uh oh!

MarcoGorelli left a comment

Uh oh!

Uh oh!

schloerke commented Mar 10, 2025

Uh oh!

schloerke commented Mar 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dangotbanned commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tracking

What type of PR is this? (check all applicable)

Related issues

Checklist

If you have comments or can explain your changes, please do so below

Uh oh!

dangotbanned Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update 1

Update 2

Uh oh!

dangotbanned commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MarcoGorelli commented Feb 27, 2025

Uh oh!

dangotbanned commented Feb 27, 2025

Uh oh!

dangotbanned Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update

Uh oh!

dangotbanned commented Feb 27, 2025

Uh oh!

MarcoGorelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

schloerke commented Mar 10, 2025

Uh oh!

schloerke commented Mar 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dangotbanned commented Feb 27, 2025 •

edited

Loading

dangotbanned commented Feb 27, 2025 •

edited

Loading

dangotbanned commented Feb 27, 2025 •

edited

Loading

dangotbanned commented Feb 27, 2025 •

edited

Loading