-
Notifications
You must be signed in to change notification settings - Fork 146
chore: remove kwargs
from selector
function
#2058
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
chore: remove kwargs
from selector
function
#2058
Conversation
@@ -83,7 +83,7 @@ def all(self: Self) -> PandasSelector: | |||
def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | |||
return [df[col] for col in df.columns] | |||
|
|||
return selector(self, func, lambda df: df.columns, {}) | |||
return selector(self, func, lambda df: df.columns) |
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.
I was planning to follow-up with replacing these lambda
s:
diff --git a/narwhals/utils.py b/narwhals/utils.py
index cb33a603..05b9cc60 100644
--- a/narwhals/utils.py
+++ b/narwhals/utils.py
@@ -10,6 +10,7 @@ from secrets import token_hex
from typing import TYPE_CHECKING
from typing import Any
from typing import Iterable
+from typing import Iterator
from typing import Sequence
from typing import TypeVar
from typing import Union
@@ -59,6 +60,7 @@ if TYPE_CHECKING:
from narwhals.typing import DataFrameLike
from narwhals.typing import DTypes
from narwhals.typing import IntoSeriesT
+ from narwhals.typing import NativeFrame
from narwhals.typing import SizeUnit
from narwhals.typing import SupportsNativeNamespace
from narwhals.typing import TimeUnit
@@ -1303,6 +1305,14 @@ def dtype_matches_time_unit_and_time_zone(
)
+def get_columns(df: NativeFrame) -> Sequence[str]:
+ return df.columns
+
+
+def iter_columns(df: NativeFrame) -> Iterator[str]:
+ yield from df.columns
+
+
def _hasattr_static(obj: Any, attr: str) -> bool:
sentinel = object()
return getattr_static(obj, attr, sentinel) is not sentinel
iter_columns
wouldn't be for this part, but would be usuable in lots of places
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.
cool, thanks!
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.
Great stuff @MarcoGorelli
Mentioned in (#2058 (comment)) Originally in - (028098e) - c597ba7
follow-up to #2057 - there's a lot of
kwargs
we're passing around that we don't needi'd generally like to refactor that whole mechanism (we'll need it for when we want to support
rolling_mean(n).over(group, order_by=id)
), but for a start we can remove them when they're not neededWhat type of PR is this? (check all applicable)
Related issues
Checklist
If you have comments or can explain your changes, please do so below