Skip to content
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(python,rust!): Infer values columns in DataFrame.pivot when values is None #14477

Merged
merged 8 commits into from
Feb 25, 2024

Conversation

mcrumiller
Copy link
Contributor

@mcrumiller mcrumiller commented Feb 14, 2024

Resolves #12087.

I think we hold off until the next breaking release.

@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars labels Feb 14, 2024
@stinodego stinodego changed the title feat: Make values parameter in pl.pivot optional feat!: Make values parameter in DataFrame.pivot optional Feb 14, 2024
@github-actions github-actions bot added the breaking Change that breaks backwards compatibility label Feb 14, 2024
@stinodego stinodego changed the title feat!: Make values parameter in DataFrame.pivot optional feat: Make values parameter in DataFrame.pivot optional Feb 14, 2024
@stinodego stinodego removed the breaking Change that breaks backwards compatibility label Feb 14, 2024
@stinodego stinodego changed the title feat: Make values parameter in DataFrame.pivot optional feat(python,rust!): Make values parameter in DataFrame.pivot optional Feb 14, 2024
@github-actions github-actions bot added the breaking rust Change that breaks backwards compatibility for the Rust crate label Feb 14, 2024
@stinodego
Copy link
Member

I think we hold off until the next breaking release.

Yes, this is breaking. You can remove the deprecation decorator from the method too.

@stinodego stinodego changed the title feat(python,rust!): Make values parameter in DataFrame.pivot optional feat!: Make values parameter in DataFrame.pivot optional Feb 14, 2024
@stinodego stinodego removed the breaking rust Change that breaks backwards compatibility for the Rust crate label Feb 14, 2024
@github-actions github-actions bot added the breaking Change that breaks backwards compatibility label Feb 14, 2024
@stinodego stinodego added this to the 1.0.0 milestone Feb 14, 2024
@mcrumiller
Copy link
Contributor Author

@stinodego got a Chocolatey CI error here, should keep an eye open to see if it pops up elsewhere.

@mcrumiller mcrumiller marked this pull request as ready for review February 14, 2024 17:32
@stinodego
Copy link
Member

@stinodego got a Chocolatey CI error here, should keep an eye open to see if it pops up elsewhere.

This is a known issue - I reported it here: ts-graphviz/setup-graphviz#557

@mcrumiller mcrumiller marked this pull request as draft February 21, 2024 13:45
@mcrumiller mcrumiller marked this pull request as ready for review February 21, 2024 19:07
@mcrumiller
Copy link
Contributor Author

@stinodego I think we should revert the set usage, unless you can think of a better way out. The order of the output columns in pivot is now unpredictable. I suppose we could sort them to make the output predictable? Preserving the order would be nice though.

@stinodego
Copy link
Member

@stinodego I think we should revert the set usage, unless you can think of a better way out. The order of the output columns in pivot is now unpredictable. I suppose we could sort them to make the output predictable? Preserving the order would be nice though.

Ah, I hadn't considered that. Let's revert it then 👍

@mcrumiller mcrumiller marked this pull request as draft February 22, 2024 00:30
@mcrumiller
Copy link
Contributor Author

mcrumiller commented Feb 22, 2024

Ah, I hadn't considered that. Let's revert it.

I have a way to both use the faster sets path and preserve column order.

@mcrumiller mcrumiller force-pushed the pivot-values branch 2 times, most recently from 9d63ac5 to a5e73c6 Compare February 22, 2024 00:40
@mcrumiller mcrumiller marked this pull request as ready for review February 22, 2024 18:04
@stinodego stinodego changed the title feat!: Make values parameter in DataFrame.pivot optional feat(python,rust!): Make values parameter in DataFrame.pivot optional Feb 25, 2024
@stinodego stinodego removed the breaking Change that breaks backwards compatibility label Feb 25, 2024
@github-actions github-actions bot added the breaking rust Change that breaks backwards compatibility for the Rust crate label Feb 25, 2024
@stinodego
Copy link
Member

stinodego commented Feb 25, 2024

I have a way to both use the faster sets path and preserve column order.

I don't think it's worth allocating a new set here. Let's just go with the initial logic which was simplest. We can further optimize this later if required.

I made the update and this is now good to merge 👍

@mcrumiller
Copy link
Contributor Author

Thanks for the help @stinodego!

@stinodego stinodego changed the title feat(python,rust!): Make values parameter in DataFrame.pivot optional feat(python,rust!): Infer values columns in DataFrame.pivot when values is None Feb 25, 2024
@stinodego stinodego merged commit 3074428 into pola-rs:main Feb 25, 2024
26 of 28 checks passed
@mcrumiller mcrumiller deleted the pivot-values branch February 26, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking rust Change that breaks backwards compatibility for the Rust crate enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make values in pl.pivot() optional
2 participants