-
Notifications
You must be signed in to change notification settings - Fork 130
feat[vector]: Eq #5681
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[vector]: Eq #5681
Conversation
Codecov Report❌ Patch coverage is ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
461c54b to
da89aa2
Compare
| // Compare offsets and sizes at valid positions | ||
| (0..len).all(|i| { | ||
| !validity.value(i) | ||
| || (self_offsets.get_as::<usize>(i) == other_offsets.get_as::<usize>(i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want usize or u64 here, given that we want to run on wasm32?
| self.views | ||
| .iter() | ||
| .zip(other.views.iter()) | ||
| .enumerate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should also have a test
0ax1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved but double check on having all complex eq fns being unit tested
CodSpeed Performance ReportMerging #5681 will improve performances by 10.94%Comparing Summary
Benchmarks breakdown
Footnotes
|
# Conflicts: # vortex-vector/src/binaryview/types.rs # vortex-vector/src/listview/scalar.rs # vortex-vector/src/struct_/scalar.rs
18d137e to
197266d
Compare
No description provided.