-
Notifications
You must be signed in to change notification settings - Fork 124
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
feat: add LazyFrame.unpivot
for spark and duckdb
#1890
Conversation
if variable_name == "": | ||
msg = "`variable_name` cannot be empty string for duckdb backend." | ||
raise NotImplementedError(msg) | ||
|
||
if value_name == "": | ||
msg = "`value_name` cannot be empty string for duckdb backend." | ||
raise NotImplementedError(msg) |
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 tried, but it is not of duckdb liking to support these
thanks, can you rebase please? |
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.
awesome work, thanks!
@@ -312,3 +312,21 @@ def join( | |||
return self._from_native_frame( | |||
self_native.join(other, on=left_on, how=how).select(col_order) | |||
) | |||
|
|||
def unpivot( |
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.
wow, so simple!
narwhals/dataframe.py
Outdated
) | ||
|
||
if on_ is None: | ||
on_ = [c for c in self.collect_schema().names() if c not in index_] |
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.
@MarcoGorelli by pushing one level up, we (might) need column names π
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.
ah sorry i don't think we need to push this one, just the str | list[str]
ones
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.
thanks @FBruzzesi ! feel free to merge on green
got a question which occurred to me, we can always simplify later
variable_name = variable_name if variable_name is not None else "variable" | ||
value_name = value_name if value_name is not None else "value" |
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 don't understand why the signature isn't variable_name: str = "variable"
in Polars, am I missing something?
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.
Good point! Not sure why they do it like that
What type of PR is this? (check all applicable)
Checklist
If you have comments or can explain your changes, please do so below