docs/tests: fix rustdoc links and appease clippy strict lints #1576
Conversation
| { | ||
| /// Checks if the permutation is correct | ||
| pub fn from_indices(v: Vec<usize>) -> Result<Self, ()> | ||
| pub fn from_indices(v: Vec<usize>) -> Option<Self> |
There was a problem hiding this comment.
I understand that you didn't "change" anything, this is mostly reformatting, but this one here is different. This is a public function, so there might be several callers out there who will be surprised by this API change.
I'm not against changing it. I guess a Result<X, ()> is functionally equivalent to an Option<X>. I just want to highlight this change.
There was a problem hiding this comment.
Personally I'm ok with this, but I'm glad you brought it up
There was a problem hiding this comment.
Let me know if you would like me to revert it.
https://effective-rust.com/transform.html
I think its better to return the Option and makes the signature clearer, and should require minimal refactoring downstream. While Result<T, ()> is basically the same functionally as Option<T> it doesn't covey the intent clearly. Usually Result is reserved for situations like where there is an expected value or an error variant and allows for propagating errors with ?.
There was a problem hiding this comment.
Agreed that it's the better practice, just good for us to keep in mind it will have to be a breaking change release.
There was a problem hiding this comment.
Ok would you like to retain it in this PR? Please let me know or merge after I make the other changes we discussed
|
@RPG-Alex same question as before, is this just another round of |
If you want to add it to the CI then be aware that the strictest setting is still sending errors: error: this range is empty so it will yield no values
--> benches/bench1.rs:746:35
|
746 | let a = a.slice_mut(s![1..-1, 1..-1]);
| ^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
help: consider using the following if you are attempting to iterate over this range in reverse
|
746 - let a = a.slice_mut(s![1..-1, 1..-1]);
746 + let a = a.slice_mut(s![1..-1, (-1..1).rev()]);This one is odd and I was considering opening an isuse about these tests as |
3 participants
Tighten linting; fix rustdoc warnings across feature gates
I continued using stricter linting and found several odd cases in tests and
examples, along with rustdoc warnings triggered by feature-gated items.
This commit cleans up those warnings by:
linking to non-exported items
All tests are still passing.
Clippy still warns about the use of
1..-1as an empty range; this appearsintentional in ndarray slicing semantics, so I’ve left it unchanged for now.
Please review the rustdoc changes that rely on feature-gated documentation.