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] support units for estimated_size ("mb","gb", etc) #4499

Merged
merged 2 commits into from
Aug 19, 2022
Merged

feat[python] support units for estimated_size ("mb","gb", etc) #4499

merged 2 commits into from
Aug 19, 2022

Conversation

alexander-beedie
Copy link
Collaborator

Added convenience support for returning estimated_size() in the units of your choice (default remains bytes):

Examples:

# integrated with series...
series.estimated_size("kb")
series.estimated_size("mb")

# ...and dataframe
frame.estimated_size("gb")
frame.estimated_size("tb")

@github-actions github-actions bot added the python Related to Python Polars label Aug 19, 2022
@alexander-beedie alexander-beedie changed the title feat[python] support units for estimated_size ('mb','gb', etc) feat[python] support units for estimated_size ("mb","gb", etc) Aug 19, 2022
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.

Nice utility, but shouldn't we try to implement these things on the Rust side? I'd rather keep our Python bindings as skinny as possible.

@ritchie46
Copy link
Member

Nice utility, but shouldn't we try to implement these things on the Rust side? I'd rather keep our Python bindings as skinny as possible.

I sometimes argue the other way, as in Rust we have to pay with compile times and binary size. I think rust users are fine with dealing with bytes, and I don't think it is worth an extra enum on the rust side.

If rust compiled fast, I certainly agreed with you.

@alexander-beedie
Copy link
Collaborator Author

alexander-beedie commented Aug 19, 2022

I think rust users are fine with dealing with bytes, and I don't think it is worth an extra enum on the rust side.

Pretty much my thoughts; you're only likely to want to change the units when interactively eyeballing a frame in python, rather than wanting to compile a unit-scaled float (vs the usual bytes integer in Rust).

@ritchie46
Copy link
Member

Thanks @alexander-beedie and thanks for the review @stinodego .

@ritchie46 ritchie46 merged commit de93b31 into pola-rs:master Aug 19, 2022
@alexander-beedie alexander-beedie deleted the estimated-size-units branch August 27, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants