Thanks. As a next step I will try to implement the interface with only the matrix as input (not considering shuffle and groups). Obviously all the documentation will be cleaned up and fixed after the implementation is satisfactory.

@VolodymyrOrlov can you specify the meaning of this assignment for me please?

I think the idea is to make sure that total number of samples from all folds equals total number of samples. Here is an example

without this line we have only 2 + 2 + 2 = 6 samples in all folds. If you uncomment this line and run the code again total number of samples equal number of samples in X, 3 + 2 + 2 = 7:

Thanks for the explanation.

Please check that test_indices works as expected

Ready for review 👍

@Mec-iS the code looks good! Couple of things left before I can proceed to merging this PR:

1. Looks like our implementation does not split data the same way as Scikit-learn's does.
2. Arguably flag shuffle is the most important feature of K-fold split. Let's add it and set it to true by default.

I had problem understanding the shape of the matrix, in my understanding:

• M::shape().0 returns the dimensions (columns) the samples
• M::shape().1 returns the samples (rows) the dimensions

Please provide guidance. I couldn't find this information clearly stated in the documentation.

ahah, sorry I got the dimensions confused. I just had to switch which index in shapes I was using and now the tests seem to pass.

Lesson learned: the developer may not know (or have forgotten, like in my case) the conventions used by numpy, so having the basics explained in the documentation for Matrix would be nice.

Question: in my last commit I added this line to explicitly define return_values as immutable before returning ownership, the same for indices in test_indices after the optional shuffle; is this worth it or the compiler can do this kind of optimisation automatically? I couldn't find a reference to this in Rust doc so I am not sure. If not, let me know I will revert the commit.

I had problem understanding the shape of the matrix, in my understanding:

• M::shape().0 returns the dimensions (columns) the samples
• M::shape().1 returns the samples (rows) the dimensions

Please provide guidance. I couldn't find this information clearly stated in the documentation.

Good catch, thanks for pointing it out! I'll fix documentation of the shape method

Question: in my last commit I added this line to explicitly define return_values as immutable before returning ownership, the same for indices in test_indices after the optional shuffle; is this worth it or the compiler can do this kind of optimisation automatically? I couldn't find a reference to this in Rust doc so I am not sure. If not, let me know I will revert the commit.

No, it is not necessary since you pass ownership of the return_values to the caller. A function that calls the method test_indices will be able to modify return_values and I see no reason why we should not let it to do so.

I am sure you've read this book but will leave this link here in case you haven't: https://doc.rust-lang.org/book/ch04-00-understanding-ownership.html.

since you pass ownership of the return_values to the caller. A function that calls the method test_indices will be able to modify return_values and I see no reason why we should not let it to do so.

yes but I dont see the necessity of passing the ownership of a mutable vector. It could be an interface leak to allow mutability on something that is not meant to be modified. Once the indices and the splits are defined, there is no reason for the developer-user to be able to modify it. That is why is conceptually more correct to return an immutable maybe.

since you pass ownership of the return_values to the caller. A function that calls the method test_indices will be able to modify return_values and I see no reason why we should not let it to do so.

yes but I dont see the necessity of passing the ownership of a mutable vector. It could be an interface leak to allow mutability on something that is not meant to be modified. Once the indices and the splits are defined, there is no reason for the developer-user to be able to modify it. That is why is conceptually more correct to return an immutable maybe.

I agree with you conceptually but I don't know an easy way to do it without wrapping your return value into another struct that will prevent a caller from modifying it. https://users.rust-lang.org/t/how-to-return-an-immutable-reference-of-a-mutable-vector/30773/5

I think wrapping return value into protective shell is an overkill

... REDACTED ...

EDIT: ok, forget it. I didn't consider that the accessibility is defined by the caller, not the callee 🥇