Skip to content

Conversation

raunakab
Copy link
Contributor

Overview

This PR adds more iterators to daft-core datatypes.

Copy link

codspeed-hq bot commented Dec 10, 2024

CodSpeed Performance Report

Merging #3539 will degrade performances by 26.61%

Comparing feat/more-iters (cae69a8) with main (d103573)

Summary

❌ 1 regressions
✅ 16 untouched benchmarks

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

Benchmarks breakdown

Benchmark main feat/more-iters Change
test_iter_rows_first_row[100 Small Files] 173.6 ms 236.6 ms -26.61%

Copy link

codecov bot commented Dec 10, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.70%. Comparing base (e5ea73f) to head (cae69a8).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/daft-core/src/array/iterator.rs 75.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3539      +/-   ##
==========================================
+ Coverage   76.11%   77.70%   +1.58%     
==========================================
  Files         710      710              
  Lines       89196    86943    -2253     
==========================================
- Hits        67894    67557     -337     
+ Misses      21302    19386    -1916     
Files with missing lines Coverage Δ
src/daft-core/src/array/iterator.rs 66.66% <75.00%> (-33.34%) ⬇️

... and 22 files with indirect coverage changes

Copy link
Member

@kevinzwang kevinzwang left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe you could even put these into a macro, seems like there's a good amount of boilerplate

@raunakab
Copy link
Contributor Author

raunakab commented Dec 11, 2024

LGTM. Maybe you could even put these into a macro, seems like there's a good amount of boilerplate

@kevinzwang Yes, good point! I'll make those edits right now and get a re-review from you.

@raunakab raunakab enabled auto-merge (squash) December 11, 2024 01:41
@raunakab raunakab merged commit f23ee37 into main Dec 11, 2024
40 of 41 checks passed
@raunakab raunakab deleted the feat/more-iters branch December 11, 2024 02:00
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