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(rust): Prefer ParquetError::oos to ParquetError::OutOfSpec #17314

Merged
merged 4 commits into from
Jul 1, 2024

Conversation

SeanTater
Copy link
Contributor

There are mixed uses of ParquetError::oos and ParquetError::OutOfSpec where it would just be clearer to pick one. So I favored the convenience constructor that saves a few keystrokes. I didn't make changes to the tests because that requires making pub fn oos(..) and I am not sure if people want that API to be exposed. If we made that change, there would be a few dozen more references cleaned up.

(This is my first commit to Polars - I'm doing some minor refactors to get my bearings so I can implement some improvements I need at work)

Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.

Project coverage is 80.74%. Comparing base (59d2529) to head (9d73ef3).
Report is 1 commits behind head on main.

Files Patch % Lines
...tes/polars-parquet/src/parquet/read/compression.rs 0.00% 4 Missing ⚠️
...es/polars-parquet/src/parquet/schema/types/spec.rs 0.00% 4 Missing ⚠️
crates/polars-parquet/src/parquet/compression.rs 0.00% 1 Missing ⚠️
...tes/polars-parquet/src/parquet/read/page/reader.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #17314   +/-   ##
=======================================
  Coverage   80.73%   80.74%           
=======================================
  Files        1475     1475           
  Lines      193238   193235    -3     
  Branches     2760     2760           
=======================================
+ Hits       156013   156018    +5     
+ Misses      36715    36707    -8     
  Partials      510      510           

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

@ritchie46
Copy link
Member

Yeap, seems fine. 👍 Thanks.

@ritchie46 ritchie46 changed the title Prefer ParquetError::oos to ParquetError::OutOfSpec Refactor: Prefer ParquetError::oos to ParquetError::OutOfSpec Jul 1, 2024
@ritchie46 ritchie46 merged commit 71e43b6 into pola-rs:main Jul 1, 2024
21 checks passed
@stinodego stinodego changed the title Refactor: Prefer ParquetError::oos to ParquetError::OutOfSpec refactor(rust): Prefer ParquetError::oos to ParquetError::OutOfSpec Jul 1, 2024
@github-actions github-actions bot added internal An internal refactor or improvement rust Related to Rust Polars labels Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants