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

fix(rust): Fix UnitVec inline clone and with_capacity #18586

Merged
merged 6 commits into from
Sep 6, 2024

Conversation

nameexhaustion
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Sep 6, 2024
@nameexhaustion nameexhaustion force-pushed the idx-vec-clone branch 2 times, most recently from 13ea45b to 9bb7f0d Compare September 6, 2024 10:48
@nameexhaustion nameexhaustion marked this pull request as ready for review September 6, 2024 11:38
Copy link

codspeed-hq bot commented Sep 6, 2024

CodSpeed Performance Report

Merging #18586 will not alter performance

Comparing nameexhaustion:idx-vec-clone (6f4e2cd) with main (f493f37)

Summary

✅ 37 untouched benchmarks

@nameexhaustion
Copy link
Collaborator Author

test

crates/polars-utils/src/idx_vec.rs Outdated Show resolved Hide resolved
crates/polars-utils/src/idx_vec.rs Outdated Show resolved Hide resolved
crates/polars-utils/src/idx_vec.rs Outdated Show resolved Hide resolved
@nameexhaustion nameexhaustion marked this pull request as draft September 6, 2024 14:08
@nameexhaustion nameexhaustion changed the title fix: Fix UnitVec inline clone fix: Fix UnitVec inline clone and with_capacity Sep 6, 2024
@nameexhaustion nameexhaustion marked this pull request as ready for review September 6, 2024 14:23
fn realloc(&mut self, new_cap: usize) {
assert!(new_cap >= self.len);
assert!(new_cap >= self.len.max(1));
Copy link
Collaborator

@orlp orlp Sep 6, 2024

Choose a reason for hiding this comment

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

No, it needs to be bigger than 1, not bigger or equal, I'd just write new_cap => self.len && new_cap > 1 instead of being fancy with max :). Capacity == 1 is a cursed special value. Doc comment also needs update.

@nameexhaustion nameexhaustion marked this pull request as draft September 6, 2024 15:34
@nameexhaustion nameexhaustion marked this pull request as ready for review September 6, 2024 15:34
@orlp orlp merged commit dcd4195 into pola-rs:main Sep 6, 2024
17 of 19 checks passed
@orlp
Copy link
Collaborator

orlp commented Sep 6, 2024

Oh I noticed it should've been fix(rust) so it doesn't end up in the automatic changelog, oh well...

@nameexhaustion nameexhaustion changed the title fix: Fix UnitVec inline clone and with_capacity fix(rust): Fix UnitVec inline clone and with_capacity Sep 6, 2024
@nameexhaustion nameexhaustion removed the python Related to Python Polars label Sep 6, 2024
@nameexhaustion nameexhaustion added internal An internal refactor or improvement rust Related to Rust Polars and removed rust Related to Rust Polars internal An internal refactor or improvement labels Sep 6, 2024
@c-peters c-peters added the accepted Ready for implementation label Sep 9, 2024
@nameexhaustion nameexhaustion deleted the idx-vec-clone branch September 11, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation fix Bug fix rust Related to Rust Polars
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants