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

[CHORE] Implement thiserror::Error for DaftError and arrow2::Error #2785

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

raunakab
Copy link
Contributor

@raunakab raunakab commented Sep 4, 2024

Overview

  • implemented thiserror::Error for common::error::DaftError and arrow2::error::Error
  • removed many impl From<X> for DaftError and many impl From<X> for arrow2::Error

Copy link

codspeed-hq bot commented Sep 4, 2024

CodSpeed Performance Report

Merging #2785 will degrade performances by 33.98%

Comparing ronnie/better-errors (a38e4a7) with main (734c13f)

Summary

❌ 1 regressions
✅ 15 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main ronnie/better-errors Change
test_show[100 Small Files] 49.6 ms 75.2 ms -33.98%

- this solves a couple of tests, since they relied on the output being strings produced from display printouts instead of debug printouts
@raunakab raunakab marked this pull request as draft September 4, 2024 18:11
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@a97d871). Learn more about missing BASE report.
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2785   +/-   ##
=======================================
  Coverage        ?   63.24%           
=======================================
  Files           ?     1002           
  Lines           ?   113729           
  Branches        ?        0           
=======================================
  Hits            ?    71929           
  Misses          ?    41800           
  Partials        ?        0           
Files with missing lines Coverage Δ
src/arrow2/src/error.rs 10.00% <100.00%> (ø)
src/common/error/src/error.rs 100.00% <100.00%> (ø)
src/common/error/src/python.rs 66.66% <ø> (ø)
src/daft-core/src/array/ops/utf8.rs 93.12% <100.00%> (ø)
src/parquet2/src/error.rs 40.00% <100.00%> (ø)

@raunakab raunakab marked this pull request as ready for review September 4, 2024 22:31
Copy link
Collaborator

@universalmind303 universalmind303 left a comment

Choose a reason for hiding this comment

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

lgtm!

@raunakab raunakab merged commit 7fe3dbc into main Sep 4, 2024
36 of 37 checks passed
@raunakab raunakab deleted the ronnie/better-errors branch September 4, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants