-
Notifications
You must be signed in to change notification settings - Fork 166
feat: introduce (experimental) plugin system #2978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 71 commits
6d36b4e
1062d99
8d4cda6
cfd156f
e3a2f3b
f65d7bf
a133796
eee9068
c3a8c4d
ca01346
c4611fe
90ad973
76232df
ebc1a8f
15d9c8c
16832f9
9e11a8f
d8663d8
34e7fb6
8a2b019
f92cdbf
54509d2
941edc4
72a5df4
e783af7
4cf2f96
a52a6bb
a4e539a
aca8cc4
02d544a
0952cd9
de0e5ce
9dfcd09
b999e9a
3460f4e
1c246dc
ab8303d
a8501f5
19dc900
42f2df8
d6a384a
9cf290d
bdd71e7
bde288f
7035533
4868aab
afd8620
b09d3ad
1a73ab8
1481d48
59589c1
6888f78
8f22fdb
b3f2b60
422ec70
b0b524b
0960d69
3207de1
d951220
ea28de2
b563ace
9847793
1f38e31
5dee0bb
c508ab0
f2d6e57
6c65fd4
b26d7d4
22362ad
d22f046
028e473
87e67bf
5c16c7f
e00c7c6
0c69762
f7765e8
e3e5ec7
0cd5f2f
d511fc0
6dd4d1c
8a96d84
1944e47
ce712e0
7bcec61
9bbc0a6
0f65066
8ad25ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,16 @@ | |
is_native_polars, | ||
is_native_spark_like, | ||
) | ||
from narwhals._utils import Implementation, Version, has_native_namespace | ||
from narwhals._utils import ( | ||
Implementation, | ||
Version, | ||
_is_native_plugin, | ||
discover_entrypoints, | ||
has_native_namespace, | ||
is_compliant_dataframe, | ||
is_compliant_lazyframe, | ||
is_compliant_series, | ||
) | ||
from narwhals.dependencies import ( | ||
get_dask_expr, | ||
get_numpy, | ||
|
@@ -314,23 +323,65 @@ def from_native( # noqa: D417 | |
) | ||
|
||
|
||
def _translate_if_compliant( # noqa: C901,PLR0911 | ||
compliant_object: Any, | ||
*, | ||
pass_through: bool = False, | ||
eager_only: bool = False, | ||
# Interchange-level was removed after v1 | ||
eager_or_interchange_only: bool, | ||
series_only: bool, | ||
allow_series: bool | None, | ||
version: Version, | ||
) -> Any: | ||
if is_compliant_dataframe(compliant_object): | ||
if series_only: | ||
if not pass_through: | ||
msg = "Cannot only use `series_only` with dataframe" | ||
raise TypeError(msg) | ||
return compliant_object | ||
return version.dataframe( | ||
compliant_object.__narwhals_dataframe__()._with_version(version), level="full" | ||
) | ||
if is_compliant_lazyframe(compliant_object): | ||
if series_only: | ||
if not pass_through: | ||
msg = "Cannot only use `series_only` with lazyframe" | ||
raise TypeError(msg) | ||
return compliant_object | ||
if eager_only or eager_or_interchange_only: | ||
if not pass_through: | ||
msg = "Cannot only use `eager_only` or `eager_or_interchange_only` with lazyframe" | ||
raise TypeError(msg) | ||
return compliant_object | ||
return version.lazyframe( | ||
compliant_object.__narwhals_lazyframe__()._with_version(version), level="full" | ||
) | ||
if is_compliant_series(compliant_object): | ||
if not allow_series: | ||
if not pass_through: | ||
msg = "Please set `allow_series=True` or `series_only=True`" | ||
raise TypeError(msg) | ||
return compliant_object | ||
return version.series( | ||
compliant_object.__narwhals_series__()._with_version(version), level="full" | ||
) | ||
# Object wasn't compliant, can't translate here. | ||
return None | ||
|
||
|
||
def _from_native_impl( # noqa: C901, PLR0911, PLR0912, PLR0915 | ||
native_object: Any, | ||
*, | ||
pass_through: bool = False, | ||
eager_only: bool = False, | ||
# Interchange-level was removed after v1 | ||
eager_or_interchange_only: bool = False, | ||
series_only: bool = False, | ||
allow_series: bool | None = None, | ||
eager_or_interchange_only: bool, | ||
series_only: bool, | ||
allow_series: bool | None, | ||
version: Version, | ||
) -> Any: | ||
from narwhals._interchange.dataframe import supports_dataframe_interchange | ||
from narwhals._utils import ( | ||
is_compliant_dataframe, | ||
is_compliant_lazyframe, | ||
is_compliant_series, | ||
) | ||
from narwhals.dataframe import DataFrame, LazyFrame | ||
from narwhals.series import Series | ||
|
||
|
@@ -350,38 +401,18 @@ def _from_native_impl( # noqa: C901, PLR0911, PLR0912, PLR0915 | |
raise ValueError(msg) | ||
|
||
# Extensions | ||
if is_compliant_dataframe(native_object): | ||
if series_only: | ||
if not pass_through: | ||
msg = "Cannot only use `series_only` with dataframe" | ||
raise TypeError(msg) | ||
return native_object | ||
return version.dataframe( | ||
native_object.__narwhals_dataframe__()._with_version(version), level="full" | ||
) | ||
if is_compliant_lazyframe(native_object): | ||
if series_only: | ||
if not pass_through: | ||
msg = "Cannot only use `series_only` with lazyframe" | ||
raise TypeError(msg) | ||
return native_object | ||
if eager_only or eager_or_interchange_only: | ||
if not pass_through: | ||
msg = "Cannot only use `eager_only` or `eager_or_interchange_only` with lazyframe" | ||
raise TypeError(msg) | ||
return native_object | ||
return version.lazyframe( | ||
native_object.__narwhals_lazyframe__()._with_version(version), level="full" | ||
) | ||
if is_compliant_series(native_object): | ||
if not allow_series: | ||
if not pass_through: | ||
msg = "Please set `allow_series=True` or `series_only=True`" | ||
raise TypeError(msg) | ||
return native_object | ||
return version.series( | ||
native_object.__narwhals_series__()._with_version(version), level="full" | ||
if ( | ||
translated := _translate_if_compliant( | ||
native_object, | ||
pass_through=pass_through, | ||
eager_only=eager_only, | ||
eager_or_interchange_only=eager_or_interchange_only, | ||
series_only=series_only, | ||
allow_series=allow_series, | ||
version=version, | ||
) | ||
) is not None: | ||
return translated | ||
|
||
# Polars | ||
if is_native_polars(native_object): | ||
|
@@ -534,6 +565,21 @@ def _from_native_impl( # noqa: C901, PLR0911, PLR0912, PLR0915 | |
raise TypeError(msg) | ||
return Version.V1.dataframe(InterchangeFrame(native_object), level="interchange") | ||
|
||
for entry_point in discover_entrypoints(): | ||
plugin = entry_point.load() | ||
if _is_native_plugin(native_object, plugin): | ||
compliant_namespace = plugin.__narwhals_namespace__(version=version) | ||
compliant_object = compliant_namespace.from_native(native_object) | ||
|
||
return _translate_if_compliant( | ||
compliant_object, | ||
pass_through=pass_through, | ||
eager_only=eager_only, | ||
eager_or_interchange_only=eager_or_interchange_only, | ||
series_only=series_only, | ||
allow_series=allow_series, | ||
version=version, | ||
) | ||
|
||
if not pass_through: | ||
msg = f"Expected pandas-like dataframe, Polars dataframe, or Polars lazyframe, got: {type(native_object)}" | ||
raise TypeError(msg) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
from __future__ import annotations | ||
|
||
import sys | ||
|
||
import pytest | ||
|
||
import narwhals as nw | ||
|
||
|
||
@pytest.mark.skipif(sys.version_info < (3, 10), reason="3.10+ required for entrypoints") | ||
def test_plugin() -> None: | ||
Comment on lines
+10
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This skip might be the reason behind the Either way, we should be able to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you still want this removing or is this obsolete? |
||
pytest.importorskip("test_plugin") | ||
df_native = {"a": [1, 1, 2], "b": [4, 5, 6]} | ||
lf = nw.from_native(df_native) # type: ignore[call-overload] | ||
assert isinstance(lf, nw.LazyFrame) | ||
assert lf.columns == ["a", "b"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[build-system] | ||
requires = ["hatchling"] | ||
build-backend = "hatchling.build" | ||
|
||
[project] | ||
name = "test_plugin" | ||
version = "0.1.0" | ||
|
||
[project.entry-points.'narwhals.plugins'] | ||
test-plugin = 'test_plugin' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
from __future__ import annotations | ||
|
||
from typing import TYPE_CHECKING | ||
|
||
if TYPE_CHECKING: | ||
from narwhals.utils import Version | ||
from tests.test_plugin.test_plugin.dataframe import DictFrame as DictFrame | ||
from tests.test_plugin.test_plugin.namespace import DictNamespace | ||
|
||
|
||
ym-pett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def __narwhals_namespace__(version: Version) -> DictNamespace: # noqa: N807 | ||
from tests.test_plugin.test_plugin.namespace import DictNamespace | ||
|
||
return DictNamespace(version=version) | ||
|
||
|
||
NATIVE_PACKAGE = "builtins" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
from __future__ import annotations | ||
|
||
from typing import TYPE_CHECKING, Any, TypeAlias | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MarcoGorelli I think
|
||
|
||
from narwhals._utils import ( | ||
Implementation, | ||
ValidateBackendVersion, | ||
Version, | ||
not_implemented, | ||
) | ||
from narwhals.typing import CompliantLazyFrame | ||
|
||
DictFrame: TypeAlias = dict[str, list[Any]] | ||
|
||
if TYPE_CHECKING: | ||
from typing_extensions import Self | ||
|
||
from narwhals import LazyFrame # noqa: F401 | ||
|
||
|
||
class DictLazyFrame( | ||
CompliantLazyFrame[Any, "DictFrame", "LazyFrame[DictFrame]"], # type: ignore[type-var] | ||
ValidateBackendVersion, | ||
): | ||
_implementation = Implementation.UNKNOWN | ||
|
||
def __init__(self, native_dataframe: DictFrame, *, version: Version) -> None: | ||
self._native_frame: DictFrame = native_dataframe | ||
self._version = version | ||
|
||
def __narwhals_lazyframe__(self) -> Self: | ||
return self | ||
|
||
def _with_version(self, version: Version) -> Self: | ||
return self.__class__(self._native_frame, version=version) | ||
|
||
@property | ||
def columns(self) -> list[str]: | ||
return list(self._native_frame.keys()) | ||
|
||
# Dunders | ||
__narwhals_namespace__ = not_implemented() | ||
__native_namespace__ = not_implemented() | ||
|
||
# Properties | ||
schema = not_implemented() # type: ignore[assignment] | ||
|
||
# Static | ||
_is_native = not_implemented() # type: ignore[assignment] | ||
|
||
# Helpers | ||
_iter_columns = not_implemented() | ||
|
||
# Functions | ||
aggregate = not_implemented() | ||
collect = not_implemented() | ||
collect_schema = not_implemented() | ||
drop = not_implemented() | ||
drop_nulls = not_implemented() | ||
explode = not_implemented() | ||
filter = not_implemented() | ||
from_native = not_implemented() | ||
group_by = not_implemented() | ||
head = not_implemented() | ||
join = not_implemented() | ||
join_asof = not_implemented() | ||
rename = not_implemented() | ||
select = not_implemented() | ||
simple_select = not_implemented() | ||
sink_parquet = not_implemented() | ||
sort = not_implemented() | ||
tail = not_implemented() | ||
to_narwhals = not_implemented() | ||
unique = not_implemented() | ||
unpivot = not_implemented() | ||
with_columns = not_implemented() | ||
with_row_index = not_implemented() |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangotbanned, pushed this as it's working, but still needs some cleaning up (e.g I've left your naming so you can see which solution we've implemented. I'm guessing the Plugin Protocol shouldn't be in here, I'll look back on your PR on this topic I reviewed and have a think about where it should go instead.
btw, are you happy with us using this one, or should we use the one you say is even simpler? (I worked with the one I could understand better, but that shouldn't be a barrier ;))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, re-read your #3086, until it can live in your
_native
, should I move it into_namespace
? (forgot_
above)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#3086 (comment)
Background
In
narwhals
, we've got three main conceptual layers:Skip me if you don't need a refresher π
(1) Narwhals
In the top-level modules, we define things like:
nw.DataFrame
nw.LazyFrame
nw.Series
They're directly user-facing
(2) Compliant
Under the
nw._compliant
sub-package, we define things like:CompliantDataFrame
CompliantLazyFrame
CompliantSeries
They're user-adjacent, some appear in
nw.typing
and annotations but not to the same degree as Narwhals.(3) Native
Following #3035, our typing for this level will be in
nw._native.py
:NativeDataFrame
NativeLazyFrame
NativeSeries
But we are mostly talking about concrete external types like:
pandas.DataFrame
polars.LazyFrame
pyarrow.ChunkedArray
What's that got to do with plugins?
If somebody wants to extend
narwhals
; they're going to be building a new set of Compliant classes.We can use them from Narwhals to talk to this new Native stuff.
Soooooo, I would say that a plugin is most closely related to the Compliant-level.
Suggestion
Rather than adding to
nw._native
, I think a better fit would be either:nw._compliant.plugins.py
nw.plugins.py
nw._utils.py
for now