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: Add AssertionError variant to PolarsError in polars-error #21460

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

Kevin-Patyk
Copy link
Contributor

This PR adds support for assertion errors to Polars error handling system. The assertion errors will ultimately be used in Rust when working on migrating assert_series_equal and assert_frame_equal to Rust to facilitate testing. This is the first part (of roughly 3) working on this issue: #21388

crates/polars-error/src/lib.rs:

  • Added AssertionError variant to PolarsError enum
  • Added AssertionError to implementation of Display
  • Added AssertionError to wrap_msg()
  • Added a macro for AssertionError under macro rules! polars_err

crates/polars-python/src/error.rs

  • Imported AssertionError exception
  • Added AssertionError match arm

crates/polars-python/src/exceptions.rs

  • Added AssertionError to create_exception!()

@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars labels Feb 25, 2025
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 79.99%. Comparing base (93bfd2c) to head (e3bd004).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-error/src/lib.rs 0.00% 2 Missing ⚠️
crates/polars-python/src/error.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21460      +/-   ##
==========================================
+ Coverage   79.94%   79.99%   +0.05%     
==========================================
  Files        1597     1598       +1     
  Lines      229007   229199     +192     
  Branches     2620     2620              
==========================================
+ Hits       183074   183355     +281     
+ Misses      45334    45245      -89     
  Partials      599      599              

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

@@ -408,6 +411,12 @@ on startup."#.trim_start())
$dtype,
)
};
(assert_eq = $lhs:expr, $rhs:expr) => {
$crate::polars_err!(
AssertionError: "equality assertion failed: {} != {}",

This comment was marked as resolved.

@coastalwhite coastalwhite merged commit e75542d into pola-rs:main Feb 25, 2025
23 checks passed
@ritchie46
Copy link
Member

This wasn't sufficient. We also need the python data types. See polars.exceptions.

@Kevin-Patyk
Copy link
Contributor Author

Kevin-Patyk commented Feb 26, 2025

Shall I update py-polars/polars/exceptions.py and make a new PR?

@Kevin-Patyk Kevin-Patyk deleted the feat/add_assertion_error branch February 26, 2025 10:11
@coastalwhite
Copy link
Collaborator

coastalwhite commented Feb 26, 2025

This wasn't sufficient. We also need the python data types. See polars.exceptions.

I actually think it is better to map it to the PyAssertionError from PyO3. This way you can catch it with the existing exceptions.

@Kevin-Patyk
Copy link
Contributor Author

This wasn't sufficient. We also need the python data types. See polars.exceptions.

I actually think it is better to map it to the PyAssertionError from PyO3. This way you can catch it with the existing exceptions.

If we choose to rely on PyAssertionError, then I do have to update error.rs and exception.rs files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants