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

refactor(python): Refactor Series/DataFrame.__getitem__ logic #16482

Merged
merged 22 commits into from
May 26, 2024

Conversation

stinodego
Copy link
Member

@stinodego stinodego commented May 25, 2024

This is in preparation for #4924

Changes

  • Rename DataFrame.select_at_idx in the PyO3 bindings to DataFrame.to_series to match the Python public API.
  • Update DataFrame.to_series to accept negative indices (implemented in the PyO3 bindings, just like Series.get_index_signed). Remove this bound checking from various other places.
  • Address some edge cases:
    • Handle empty list as column selector
    • Handle boolean NumPy array as column selector
  • Remove full scans of Sequence inputs (e.g. is_int_sequence), use first entry instead and fail later.
  • Add lots of tests

We should probably add some methods to the API like DataFrame.filter_columns and DataFrame.get_columns_by_index - these are only available through __getitem__ right now.

@github-actions github-actions bot added internal An internal refactor or improvement python Related to Python Polars labels May 25, 2024
@stinodego stinodego force-pushed the depr-getitem-row branch 2 times, most recently from eb856b2 to 0401a56 Compare May 25, 2024 10:08
@stinodego stinodego marked this pull request as ready for review May 25, 2024 10:09
Copy link

codecov bot commented May 25, 2024

Codecov Report

Attention: Patch coverage is 92.33871% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 81.51%. Comparing base (d265cd1) to head (05bc79d).

Files Patch % Lines
py-polars/polars/_utils/getitem.py 90.95% 11 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16482      +/-   ##
==========================================
- Coverage   81.51%   81.51%   -0.01%     
==========================================
  Files        1409     1410       +1     
  Lines      184974   185032      +58     
  Branches     2981     2981              
==========================================
+ Hits       150778   150823      +45     
- Misses      33675    33693      +18     
+ Partials      521      516       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stinodego stinodego marked this pull request as draft May 26, 2024 03:56
@stinodego stinodego marked this pull request as ready for review May 26, 2024 08:33
@stinodego stinodego merged commit 5b25fb8 into main May 26, 2024
19 checks passed
@stinodego stinodego deleted the depr-getitem-row branch May 26, 2024 10:35
@c-peters c-peters added the accepted Ready for implementation label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation internal An internal refactor or improvement python Related to Python Polars
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants