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

fix(python): Make to_pandas() work for Dataframe and Series with dtype Object #13910

Merged
merged 11 commits into from
Jan 26, 2024

Conversation

itamarst
Copy link
Contributor

Fixes #13021

Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I left a bunch of small comments.

py-polars/polars/series/series.py Outdated Show resolved Hide resolved
py-polars/tests/unit/namespaces/test_plot.py Show resolved Hide resolved
py-polars/polars/series/series.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
@itamarst
Copy link
Contributor Author

@stinodego ready for another round of review, if you have a moment.

@stinodego stinodego changed the title fix(python): Make to_pandas() work for Dataframe and Series with dtype of pl.Object (#13021) fix(python): Make to_pandas() work for Dataframe and Series with dtype Object Jan 25, 2024
Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments. With that, this should be good to merge.

I am not too happy with splitting up the implementation up like this (different strategies for different columns, then merge them). The Object columns are not converted in parallel and I am not sure about the performance implications of inserting them one-by-one into a pandas dataframe (maybe this is free, not sure).

For now it seems warranted to correctly handle Object types. But I wonder what a more ideal solution would look like.

PS: Could you send me another message on Discord? I lost our previous conversation and I can't find you by your username.

py-polars/tests/unit/interop/test_to_pandas.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved
@itamarst
Copy link
Contributor Author

@stinodego back to you.

@ritchie46
Copy link
Member

Thank you @itamarst

@ritchie46 ritchie46 merged commit 79a0984 into pola-rs:main Jan 26, 2024
15 checks passed
@itamarst itamarst deleted the 13021-to_pandas_pyobject branch January 26, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_pandas() breaks Python objects in object column
4 participants