-
Notifications
You must be signed in to change notification settings - Fork 5
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
Contact duration and distance #123
Conversation
Causing problems with CI, reason unclear as it's supposed to also use MacOs This reverts commit b48a649.
c5b14b7
to
040155e
Compare
@i-schuetz Ups, I noticed its a draft PR only after I have gone through the whole thing :( |
59b336c
to
6638b99
Compare
6638b99
to
18e5f6b
Compare
18e5f6b
to
076a86b
Compare
1ed3f93
to
2e2077a
Compare
2275ba7
to
a8049aa
Compare
3b39c97
to
6f72b45
Compare
cb13cb7
to
3ff6f0a
Compare
Should be better for performance
Some(merged ) => vec![merged], | ||
None => vec![last.to_owned(), tcn] | ||
}; | ||
let mut head: Vec<ObservedTcn> = db_tcns.to_owned().into_iter().take(db_tcns.len() - 1).collect(); |
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.
We could avoid iterating over the vector and just replace the last element: https://doc.rust-lang.org/std/mem/fn.replace.html
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.
Unclear how mem::replace
helps. It seems to replace the complete vector. db_tcns
is also unlikely to contain more than a couple of elements (these are the stored, averaged exposures for an individual TCN) so the performance is negligible.
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.
mem::replace works on a vector element.
Good point regarding small vector size.
No description provided.