refactor: adds `_compliant` sub-package by dangotbanned · Pull Request #2149 · narwhals-dev/narwhals

dangotbanned · 2025-03-05T16:22:01Z

Closes #2044

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

Direct follow-up to (#2119), aiming to make space for similar changes for other Compliant* protocols.

This PR spiralled a bit, but tackles a few quite-related issues.

Lazy-only typing

#2044 has been resolved through the use of the new protocol NativeExpr.

narwhals/narwhals/_compliant/expr.py

Lines 65 to 73 in b3bdb3b

    
           class NativeExpr(Protocol): 
        
               """An `Expr`-like object from a package with [Lazy-only support](https://narwhals-dev.github.io/narwhals/extending/#levels-of-support). 
        
               Protocol members are chosen *purely* for matching statically - as they 
        
               are common to all currently supported packages. 
        
               """ 
        
               def between(self, *args: Any, **kwds: Any) -> Any: ... 
        
               def isin(self, *args: Any, **kwds: Any) -> Any: ...

`(Arrow|PandasLike)*` deduplication

All of the new classes prefixed with Eager now provide the common functionality between the two Eager-only backends.
The big one is EagerExpr

narwhals/narwhals/_compliant/expr.py

Lines 257 to 260 in b3bdb3b

    
           class EagerExpr( 
        
               CompliantExpr[EagerDataFrameT, EagerSeriesT], 
        
               Protocol38[EagerDataFrameT, EagerSeriesT], 
        
           ):

A very unexpected part of that was that they could also share *ExprNamespace implementations

(5a13b63)
(2777822)

Probably the most visual part of the PR as it removes the need for 12 modules

`*Namespace` protocols

Working on the Eager namespaces led to the new generic protocols for both (expr|series)_* namespaces https://github.com/narwhals-dev/narwhals/blob/b3bdb3b3ba52dada9cadde3c50d68520f1270a09/narwhals/_compliant/any_namespace.py

These can be re-used for all backends 😄

Misc

I followed up on (#2055 (comment)) and converted some methods to simpler .from_ variants.
The only remaining one (_create_compliant_series) is discussed in (#2149 (comment))

I also opened (#2169) for following up some strange pandas behavior

Note

There might be some other bits in commit descriptions I've missed

Follow-up to #2119 (comment)

- Long-term, should probably be defined in `nw.typing` - Or just generally used in parts of each backend impl #2064 (comment)

#2044 (comment)

narwhals/typing.py

narwhals/_compliant/expr.py

https://github.com/narwhals-dev/narwhals/actions/runs/13680592205/job/38251688439

#2149 (comment)

Fixes #2044

dangotbanned · 2025-03-06T17:24:15Z

narwhals/_compliant/typing.py

+CompliantSeriesOrNativeExprT_co = TypeVar(
+    "CompliantSeriesOrNativeExprT_co",
+    bound="CompliantSeries | NativeExpr",
+    covariant=True,
+)


Really want something that rolls off the tongue better than this 🤔

```log error: All protocol members must have explicitly declared types [misc] ```

narwhals/_compliant/expr.py

Beginning to merge parts of (#2055)

narwhals/_compliant/expr.py

dangotbanned · 2025-03-10T09:46:20Z

Still managing to get more LOC (and module) reductions in (40d718f) 😉

- Forgot that I did that in the other PR - Important as it changes the variance of `EagerSeries` https://github.com/narwhals-dev/narwhals/blob/35cef0b1e2c892fb24aa730902b08b6994008c18/narwhals/_protocols.py#L50-L64

MarcoGorelli · 2025-03-10T18:44:18Z

thanks - gonna review/merge tomorrow 👍

dangotbanned · 2025-03-10T19:19:49Z

thanks - gonna review/merge tomorrow 👍

perfect, thanks @MarcoGorelli

narwhals/_compliant/expr.py

narwhals/_compliant/namespace.py

#2149 (comment)

The points I'm trying to make are: - the method isn't self-explanatory from its name - `Find All References` won't show every use - This will be replaced soon - but still gathering the details on how/with what

dangotbanned · 2025-03-11T09:16:30Z

Thanks for the review @MarcoGorelli - sorry that this one spiralled!

pyright failures are not related

https://github.com/eakmanrq/sqlframe/releases/tag/v3.24.0
Also showing on this branch

pytest failures also seems to be sqlframe, but only an [XPASS(strict)]

MarcoGorelli · 2025-03-11T09:49:59Z

i've put up a PR for the xfail, but I think we need to resolve the pyright failures (in a separate PR?) before merging this if that's OK

this could be a chance to address #2185 too

dangotbanned · 2025-03-11T10:13:37Z

#2149 (comment)

All good - taking a look now

Notes (Updating)

24 errors
only impacts _spark_like.(dataframe|group_by)

MarcoGorelli · 2025-03-11T14:48:25Z

narwhals/_compliant/namespace.py

+    @deprecated(
+        "Internally used for `numpy.ndarray` -> `CompliantSeries`\n"
+        "Also referenced in untyped `nw.dataframe.DataFrame._extract_compliant`\n"
+        "See Also:\n"
+        "  - https://github.com/narwhals-dev/narwhals/pull/2149#discussion_r1986283345\n"
+        "  - https://github.com/narwhals-dev/narwhals/issues/2116\n"
+        "  - https://github.com/narwhals-dev/narwhals/pull/2169"
+    )
+    def _create_compliant_series(self, value: Any) -> EagerSeriesT_co: ...


why is this deprecated?

"Deprecated" is probably the wrong word.

I'm trying to mark it as something I want to replace with something like:

CompliantNamespace._series.from_numpy CompliantNamespace._series.from_iterable

There are still some parts I wanna clear up first in the linked issues - hoping to explore that more in a future PR 🤞

I don't think the current name is helpful in hinting that it is used for numpy - or at least it surprised me when I followed through the references

cool, thanks! ok to move forwards then

New usage of the method will show up in VSCode with a strikethrough and that message on hover

But there isn't a runtime effect - purely for internal docs

MarcoGorelli

thanks @dangotbanned , great refactor here!

approving so we can move forwards with other work, I just left a question about why an internal method is deprecated

dangotbanned · 2025-03-11T15:19:15Z

Thanks again for making it through this one @MarcoGorelli 😍

dangotbanned added 4 commits March 5, 2025 15:10

refactor: Add _compliant sub-package

931d7ea

Follow-up to #2119 (comment)

refactor: migrate imports, delete _selectors

cef630e

refactor: Export Eval(Names|Series)

ca2ca24

- Long-term, should probably be defined in `nw.typing` - Or just generally used in parts of each backend impl #2064 (comment)

feat(DRAFT): Add placeholder (Eager|Lazy)Expr protocols

9cc3ce8

#2044 (comment)

dangotbanned added the internal label Mar 5, 2025

dangotbanned commented Mar 5, 2025

View reviewed changes

narwhals/typing.py Show resolved Hide resolved

dangotbanned commented Mar 5, 2025

View reviewed changes

narwhals/_compliant/expr.py Outdated Show resolved Hide resolved

dangotbanned added 7 commits March 5, 2025 16:37

ci: ignore coverage

53413ae

https://github.com/narwhals-dev/narwhals/actions/runs/13680592205/job/38251688439

Merge remote-tracking branch 'upstream/main' into compliant-package

9efd3c6

refactor: avoid adding typing.IntoCompliantExpr

2bf05fb

#2149 (comment)

refactor: avoid adding typing.CompliantFrameT

7fb4914

#2149 (comment)

refactor: avoid adding typing.CompliantSeriesT_co

51820a1

#2149 (comment)

refactor: avoid adding typing.CompliantNamespace

8f1c5ca

#2149 (comment)

refactor: avoid adding typing.CompliantExpr

5bf2bdb

#2149 (comment)

dangotbanned changed the title ~~refactor(DRAFT): adds _compliant sub-package~~ refactor: adds _compliant sub-package Mar 6, 2025

dangotbanned marked this pull request as ready for review March 6, 2025 16:04

fix(typing): Resolve NativeExpr-related issues

075228b

Fixes #2044

dangotbanned marked this pull request as draft March 6, 2025 17:22

dangotbanned linked an issue Mar 6, 2025 that may be closed by this pull request

fix TypeVar used in (SparkLike|DuckDB)Expr #2044

Closed

dangotbanned commented Mar 6, 2025

View reviewed changes

dangotbanned added fix typing labels Mar 6, 2025

dangotbanned added 2 commits March 6, 2025 17:37

refactor(typing): spec out and reuse LazyExpr

a0fe52a

be quiet mypy!

23fdf8f

```log error: All protocol members must have explicitly declared types [misc] ```

dangotbanned commented Mar 6, 2025

View reviewed changes

narwhals/_compliant/expr.py Show resolved Hide resolved

dangotbanned added 4 commits March 6, 2025 20:29

feat(DRAFT): EagerExpr from _expression_parsing.py

d58e2c9

Beginning to merge parts of (#2055)

feat: Generic dunders

6cb725c

chore(typing): fix variance issues

bc93782

feat(DRAFT): add a couple more methods

7d48726

dangotbanned commented Mar 6, 2025

View reviewed changes

narwhals/_compliant/expr.py Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into compliant-package

1338d34

dangotbanned added 2 commits March 10, 2025 10:13

Merge branch 'main' into compliant-package

c7ba9f9

Merge branch 'main' into compliant-package

7119ad2

dangotbanned mentioned this pull request Mar 10, 2025

refactor: pass ExprMetadata down to compliant Exprs from narwhals.Expr #1848

Closed

MarcoGorelli reviewed Mar 10, 2025

View reviewed changes

narwhals/_compliant/expr.py Outdated Show resolved Hide resolved

MarcoGorelli reviewed Mar 10, 2025

View reviewed changes

narwhals/_compliant/namespace.py Outdated Show resolved Hide resolved

dangotbanned added 3 commits March 11, 2025 08:54

chore: remove expr.py notes

12da35b

#2149 (comment)

chore: remove namespace.py notes

79c7c8d

#2149 (comment)

docs: Use more helpful deprecation message

34dd94a

The points I'm trying to make are: - the method isn't self-explanatory from its name - `Find All References` won't show every use - This will be replaced soon - but still gathering the details on how/with what

dangotbanned mentioned this pull request Mar 11, 2025

ci: unxfail sqlframe #2186

Merged

10 tasks

Merge branch 'main' into compliant-package

617ebc3

dangotbanned mentioned this pull request Mar 11, 2025

chore(typing): Resolve sqlframe>=3.24.0 errors #2187

Merged

10 tasks

Merge branch 'main' into compliant-package

9b93ef8

dangotbanned requested a review from MarcoGorelli March 11, 2025 13:58

MarcoGorelli reviewed Mar 11, 2025

View reviewed changes

MarcoGorelli approved these changes Mar 11, 2025

View reviewed changes

dangotbanned merged commit 1d8bc31 into main Mar 11, 2025
28 of 29 checks passed

dangotbanned deleted the compliant-package branch March 11, 2025 15:19

This was referenced Mar 12, 2025

refactor: Add CompliantSeries.from_numpy #2196

Merged

Formalise Compliant* protocols #2230

Closed

dangotbanned mentioned this pull request May 5, 2025

Add protocols for LazyExpr*Namespace #2500

Closed

dangotbanned mentioned this pull request Jun 22, 2025

refactor: Enforce LazyExpr._from_elementwise_horizontal_op #2718

Merged

10 tasks

	class NativeExpr(Protocol):
	"""An `Expr`-like object from a package with [Lazy-only support](https://narwhals-dev.github.io/narwhals/extending/#levels-of-support).

	Protocol members are chosen purely for matching statically - as they
	are common to all currently supported packages.
	"""

	def between(self, args: Any, *kwds: Any) -> Any: ...
	def isin(self, args: Any, *kwds: Any) -> Any: ...

	class EagerExpr(
	CompliantExpr[EagerDataFrameT, EagerSeriesT],
	Protocol38[EagerDataFrameT, EagerSeriesT],
	):

Conversation

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

Uh oh!

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

Lazy-only typing

(Arrow|PandasLike)* deduplication

*Namespace protocols

Misc

Uh oh!

Uh oh!

Uh oh!

dangotbanned Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

MarcoGorelli commented Mar 10, 2025

Uh oh!

dangotbanned commented Mar 10, 2025

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

MarcoGorelli commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Notes (Updating)

Uh oh!

MarcoGorelli Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

dangotbanned Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli left a comment

Choose a reason for hiding this comment

Uh oh!

dangotbanned commented Mar 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dangotbanned commented Mar 5, 2025 •

edited

Loading

`(Arrow|PandasLike)*` deduplication

`*Namespace` protocols

dangotbanned commented Mar 10, 2025 •

edited

Loading

dangotbanned commented Mar 11, 2025 •

edited

Loading

MarcoGorelli commented Mar 11, 2025 •

edited

Loading

dangotbanned commented Mar 11, 2025 •

edited

Loading