Implemented a function for smarter debug formatting.

[andrei-papou]

PR for #398

How it works:

let arr = Array3::from_elem((15, 2, 15), 1.0);
println!("{:?}", arr);

[[[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 ...,
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0, 1.0, 1.0, ..., 1.0, 1.0, 1.0, 1.0, 1.0]]] shape=[15, 2, 15], strides=[30, 15, 1], layout=C (0x1), const ndim=3

let arr = Array3::from_elem((3, 3, 3), 1.0);
println!("{:?}", arr);

[[[1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0]],
 [[1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0],
  [1.0, 1.0, 1.0]]] shape=[3, 3, 3], strides=[9, 3, 1], layout=C (0x1), const ndim=3

The number of elements before ellipses is managed by the PRINT_ELEMENTS_LIMIT constant.

[LukeMathWalker]

Typo: to me printed

[LukeMathWalker]

The output looks quite nice!
I have left some comments pointing at a bunch of functions in std that could help in visually denoising the new function.
It would also be great to have some tests for these new routines, both for the selection of printing mode and the formatting itself.

[SuperFluffy]

I'd refer to not reexport this public constant at the top level of the crate, but rather reexport it from some config or integration_test module.

As far as I can tell the motivation for making it pub is entirely for making it usable within the integration tests at tests/format.rs?

[andrei-papou]

Fixed

[SuperFluffy]

Awesome. I think that's better. I understand your motivation to have a way of configuring formatting similar to numpy's set_printoptions, but we wouldn't be able to actually accomplish that through const PRINT_ELEMENT_LIMIT. I am actually not sure right now if that was possible at all for a user of the library to accomplish at compile time.

[SuperFluffy]

I personally don't like that the implementation detail PRINTS_ELEMENTS_LIMIT is made public. Can the integration tests at tests/format.rs be made into unit tests under arrayformat.rs? Or is there a reason to have them as integration tests?

[andrei-papou]

@SuperFluffy I can move them to arrayformat.rs. But as far as I understand to convert those tests to real unit tests I should test array_format_v2 directly, not Display, Debug, etc. implementations. And I'm not really sure how to mock f: &mut fmt::Formatter and mut format: F arguments for the array_format_v2 function. Any help here is appreciated.

Although I agree that those tests should be unit tests, it would be good to give a user some control over PRINT_ELEMENTS_LIMIT. For example, numpy allows configuring this number using set_printoptions (see edgeitems argument).

[andrei-papou]

@LukeMathWalker @SuperFluffy any more comments on the changes? I think all the previous issues are fixed

[LukeMathWalker]

Sorry for the late reply @andrei-papou, it has been a crazy week.
I will try to set aside some time this weekend to go over the PR one last time.

[LukeMathWalker]

I have opened a PR with some observations @andrei-papou.

[andrei-papou]

@LukeMathWalker I tried to make the function more modular by extracting parts of the functionality to other functions and by reducing the number of boolean flags. I also got rid of the explicit second for loop. I hope the code is a bit more readable now. Please take a look

[LukeMathWalker]

I have opened a PR on your branch, trying out a different approach to solve this issue @andrei-papou - let me know what you think of it.

[LukeMathWalker]

I am happy to merge this into master. Do you want to have a look @jturner314?

[jturner314]

@LukeMathWalker Sure, I'm happy to review it, but feel free to go ahead and merge it if it looks good to you. I won't be able to review it until late next week or so. (It's the end of the semester right now, so I'm busy with exams.) Thanks for working with @andrei-papou on this PR.

[LukeMathWalker]

I am happy to call the shots and go ahead with this one - I'd like to avoid consuming your time on PRs I am confident reviewing (we need it for all the others were I have little context 👀).
Good luck with your exams @jturner314!

[jturner314]

Thank you. :)