- Notifications
You must be signed in to change notification settings - Fork352
cleanup clippy lints#316
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| Strategy::best_score =>match task{ | ||
| Task::regression =>{ | ||
| sql +=&format!("{predicate}\nORDER BY models.metrics->>'r2' DESC NULLS LAST"); | ||
| let _ =write!( |
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.
What come on clippy, this seems totally fine.
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.
format! does an extra allocation compared to write... 🤷♂️
| Some(limit) =>Some(limit.try_into().unwrap()), | ||
| None =>None, | ||
| }; | ||
| let limit:Option<usize> = limit.map(|limit| limit.try_into().unwrap()); |
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.
Weird.
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.
yeah, turns out you can map things like this...
| match guard.get(&model_id){ | ||
| Some(data) =>{ | ||
| let bst =Booster::load_buffer(&data).unwrap(); | ||
| let bst =Booster::load_buffer(data).unwrap(); |
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 was a clippy warning? Should of been a compilation error...
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.
The compiler handles the extra dereference for you.
| }; | ||
| for iin0..limit{ | ||
| let age = diabetes.data[(i* diabetes.num_features) +0]; | ||
| let age = diabetes.data[(i* diabetes.num_features)]; |
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.
Hah!
| let target = digits.target[i]; | ||
| // shape the image in a 2d array | ||
| letmut image =vec![vec![0.;8];8]; | ||
| #[allow(clippy::needless_range_loop)]// x & y are in fact used |
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.
Oh no, don't get me started with these haha! Rubocop was plenty, thank you
| usecrate::orm::Dataset; | ||
| usecrate::orm::Task; | ||
| #[allow(clippy::type_complexity)] |
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.
Clippy needs to get it together.
| .as_str(), | ||
| ), | ||
| ) | ||
| let task =Task::from_str(&task.unwrap_or_else(||{ |
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.
Ok what's up with these. Why does not likeexpect?
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.
expect input is always executed which may have side effects or allocations. or_else is only executed if needed.
| ]) | ||
| ).first(); | ||
| if result.len() >0{ | ||
| if!result.is_empty(){ |
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.
He got you there.
| let parts =self | ||
| .relation_name | ||
| .split(".") | ||
| .split('.') |
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.
Ummm. Those are completely different data types if you ask me. Although we may have just removed a heap allocation so that's cool.
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.
yeah, so why allocate the string if you can just use a char instead...
| }; | ||
| let num_train_rows = num_rows - num_test_rows; | ||
| if num_train_rows<=0{ | ||
| if num_train_rows==0{ |
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.
Clippy never heard of off-by-one errors? == 0 always made me paranoid.
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.
it's unsigned. Can't be < 0.
No description provided.