-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Adding ndarray implementations for ArgminRandom and ArgminMinMax #231
Conversation
Thanks for the contribution! Could you have a look at the clippy lints and run rustfmt, please? Other than that it looks good to me :) |
On other thing I noticed: Could you add tests, please? I know that the other backends are also missing tests, but this could be a good place to start testing this ;) One thing I could imagine in case of |
The remaining clippy errors are due to an update of clippy. You can simply add |
Sure. I'll add a couple tests when I get some free time. |
Sounds great! The fix for the clippy lints are now in |
Codecov ReportBase: 91.67% // Head: 91.50% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #231 +/- ##
==========================================
- Coverage 91.67% 91.50% -0.17%
==========================================
Files 116 117 +1
Lines 17601 17632 +31
==========================================
Hits 16135 16135
- Misses 1466 1497 +31
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I rebased this commit and removed the ndarray ArgminMinMax implementation in favor of the one which was added to main since this PR was opened. I also implemented the trait directly for all basic types instead of using traits because this causes fewer problems down the line in my experience. I am merging this PR despite the missing tests which will be added later. Thanks again for this contribution! :) |
Adds missing trait implementations for ndarray inputs.