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.

Sign up for GitHub

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

Jump to bottom

feat!(excelreader): only load dtypes for columns specified via use_columns #329

Merged
merged 5 commits into from
Feb 19, 2025
Merged

feat!(excelreader): only load dtypes for columns specified via use_columns #329

merged 5 commits into from
Feb 19, 2025

Conversation

lukapeschke
Copy link
Collaborator

This changes our logic regarding the information we extract on sheet load. Rather than loading dtype information for all available columns, we only load it for selected columns. This introduces two breaking changes:

  • For ExcelTable and ExcelSheet, available_columns is no longer a property. It is now a method, and information regarding available columns is computed on-demand (the information is cached after having been computed once).
  • If use_columns is a callable, it no longer receives a ColumnInfo object, but a ColumnInfoBuilder instead (no dtype information).
  • ColumnInfoBuilder is now part of the Python API.

@PrettyWood If you have a better naming suggestion for ColumnInfoBuilder, I'd love to hear it. Maybe ColumnNameInfo or ColumnInfoNoDtype ?

closes #327

@lukapeschke
feat!(excelsheet,exceltable): make available_columns a method rather …
51d5c74 
…than a property

Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
refactor: impl From<FastExcelError> for PyErr
6f529f7 
This allows to return FastExcelResult<T> in pymethods

Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
feat!(excelreader): only load dtypes for columns specified via `use_c…
a274276 
…olumns`

Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke lukapeschke added bug Something isn't working enhancement New feature or request ✋ need review ✋ 🦀 rust 🦀 Pull requests that edit Rust code labels Feb 17, 2025
@lukapeschke lukapeschke self-assigned this Feb 17, 2025
PrettyWood
PrettyWood reviewed Feb 18, 2025
View reviewed changes
Copy link
Member

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

Looks great! I prefer ColumnInfoNoDtype

@PrettyWood PrettyWood merged commit bf4a229 into main Feb 19, 2025
23 checks passed
@PrettyWood PrettyWood deleted the make-selected-columns-a-method branch February 19, 2025 14:54
@lukapeschke lukapeschke mentioned this pull request Feb 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PrettyWood PrettyWood PrettyWood approved these changes

Assignees

@lukapeschke lukapeschke

Labels
bug Something isn't working 🦀 rust 🦀 Pull requests that edit Rust code enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When using use_columns it still reads other columns
2 participants
@lukapeschke @PrettyWood