Skip to content
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

Implemented a function for smarter debug formatting. #606

Merged
merged 31 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fe4365c
Implemented a function for smarter debug formatting.
Mar 24, 2019
2450f50
Fixed existing tests
Mar 24, 2019
3474d3f
Some fixes:
Mar 26, 2019
2a8a7c7
Implemented axis shrinking for n-dim arrays
Mar 26, 2019
1b5da67
All the PR issues are fixed. Tests are needed.
Mar 27, 2019
fbf8cac
Fixed the tests
Mar 27, 2019
51c4e11
Added tests for 1- and 2-dimensional array with overflow
Mar 27, 2019
0d11708
Try to fix 1.31 build failing
Mar 27, 2019
18c868a
PRINT_ELEMENTS_LIMIT is now private
Mar 28, 2019
3aa8abe
Remove dead code
LukeMathWalker Apr 16, 2019
17f86bb
Rename format_array_v2 to format_array
LukeMathWalker Apr 16, 2019
d48c5a9
Simplify code using ndim
LukeMathWalker Apr 16, 2019
6c636dc
Access last element directly
LukeMathWalker Apr 16, 2019
88d53e3
Add a test to specify behaviour for zero-dimensional arrays
LukeMathWalker Apr 16, 2019
b962a7b
Add a test for empty arrays
LukeMathWalker Apr 16, 2019
29a8f70
Test zero-length axes
LukeMathWalker Apr 16, 2019
27c4a92
We already know that ndim > 0
LukeMathWalker Apr 16, 2019
49b4338
Merge pull request #2 from LukeMathWalker/smart-debug-formatting-review
Apr 17, 2019
b535f28
Made the formatting logic more modular
Apr 17, 2019
2a8a3b0
Simplify element access
Apr 20, 2019
5fd9b2f
Almost there
Apr 20, 2019
f60185c
Clone for the win
Apr 20, 2019
198d689
Cast to 1d array
Apr 20, 2019
c1036fb
First and last elements have to be special cased. Done for multidimen…
Apr 20, 2019
5336b10
Extract in a separate function
Apr 21, 2019
4fa8a62
Tests are passing
Apr 21, 2019
e659390
Minor improvements
Apr 21, 2019
0abff2a
Formatting
Apr 21, 2019
e049abd
Remove closure
Apr 23, 2019
83c9f08
Use custom enum
Apr 23, 2019
72e05d7
Merge pull request #3 from LukeMathWalker/refactoring-smarter-debugging
Apr 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 135 additions & 6 deletions src/arrayformat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,138 @@ use super::{
Data,
Dimension,
NdProducer,
Ix
};
use crate::dimension::IntoDimension;

pub const PRINT_ELEMENTS_LIMIT: Ix = 3;

fn format_array_v2<A, S, D, F>(view: &ArrayBase<S, D>,
f: &mut fmt::Formatter,
mut format: F,
limit: Ix) -> fmt::Result
where F: FnMut(&A, &mut fmt::Formatter) -> fmt::Result,
D: Dimension,
S: Data<Elem=A>,
{
if view.shape().is_empty() {
// Handle 0-dimensional array case first
return format(view.iter().next().unwrap(), f)
}

let overflow_axes: Vec<Ix> = view.shape().iter()
.enumerate()
.rev()
.filter(|(_, axis_size)| **axis_size > 2 * limit)
.map(|(axis, _)| axis)
.collect();

let ndim = view.dim().into_dimension().slice().len();
let nth_idx_max = view.shape().iter().last().unwrap();

// None will be an empty iter.
let mut last_index = match view.dim().into_dimension().first_index() {
None => view.dim().into_dimension().clone(),
Some(ix) => ix,
};
write!(f, "{}", "[".repeat(ndim))?;
let mut first = true;
// Shows if ellipses for vertical split were printed.
let mut printed_ellipses_v = false;
// Shows if ellipses for horizontal split were printed.
let mut printed_ellipses_h = vec![false; ndim];
// Shows if the row was printed for the first time after horizontal split.
let mut no_rows_after_skip_yet = false;

// Simply use the indexed iterator, and take the index wraparounds
// as cues for when to add []'s and how many to add.
for (index, elt) in view.indexed_iter() {
let index = index.into_dimension();
let take_n = if ndim == 0 { 1 } else { ndim - 1 };
let mut update_index = false;

let skip_row_for_axis = overflow_axes.iter()
.filter(|axis| {
if **axis == ndim - 1 {
return false
};
let sa_idx_max = view.shape().iter().skip(**axis).next().unwrap();
let sa_idx_val = index.slice().iter().skip(**axis).next().unwrap();
sa_idx_val >= &limit && sa_idx_val < &(sa_idx_max - &limit)
})
.min()
.map(|v| *v);
if let Some(_) = skip_row_for_axis {
no_rows_after_skip_yet = true;
}

for (i, (a, b)) in index.slice()
.iter()
.take(take_n)
.zip(last_index.slice().iter())
.enumerate() {
if a != b {
printed_ellipses_h.iter_mut().skip(i + 1).for_each(|e| { *e = false; });

if skip_row_for_axis.is_none() {
printed_ellipses_v = false;
// New row.
// # of ['s needed
let n = ndim - i - 1;
if !no_rows_after_skip_yet {
write!(f, "{}", "]".repeat(n))?;
writeln!(f, ",")?;
}
no_rows_after_skip_yet = false;
write!(f, "{}", " ".repeat(ndim - n))?;
write!(f, "{}", "[".repeat(n))?;
} else if !printed_ellipses_h[skip_row_for_axis.unwrap()] {
let ax = skip_row_for_axis.unwrap();
let n = ndim - i - 1;
write!(f, "{}", "]".repeat(n))?;
writeln!(f, ",")?;
write!(f, "{}", " ".repeat(ax + 1))?;
writeln!(f, "...,")?;
printed_ellipses_h[ax] = true;
}
first = true;
update_index = true;
break;
}
}

if skip_row_for_axis.is_none() {
let mut print_elt = true;
let nth_idx_op = index.slice().iter().last();
if overflow_axes.contains(&(ndim - 1)) {
let nth_idx_val = nth_idx_op.unwrap();
if nth_idx_val >= &limit && nth_idx_val < &(nth_idx_max - &limit) {
print_elt = false;
if !printed_ellipses_v {
write!(f, ", ...")?;
printed_ellipses_v = true;
}
}
}

if print_elt {
if !first {
write!(f, ", ")?;
}
first = false;
format(elt, f)?;
}
}

if update_index {
last_index = index;
}
}
write!(f, "{}", "]".repeat(ndim))?;
Ok(())
}

#[allow(dead_code)]
fn format_array<A, S, D, F>(view: &ArrayBase<S, D>, f: &mut fmt::Formatter,
mut format: F)
-> fmt::Result
Expand Down Expand Up @@ -92,7 +221,7 @@ impl<'a, A: fmt::Display, S, D: Dimension> fmt::Display for ArrayBase<S, D>
where S: Data<Elem=A>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
format_array(self, f, <_>::fmt)
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)
}
}

Expand All @@ -105,7 +234,7 @@ impl<'a, A: fmt::Debug, S, D: Dimension> fmt::Debug for ArrayBase<S, D>
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// Add extra information for Debug
format_array(self, f, <_>::fmt)?;
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)?;
write!(f, " shape={:?}, strides={:?}, layout={:?}",
self.shape(), self.strides(), layout=self.view().layout())?;
match D::NDIM {
Expand All @@ -124,7 +253,7 @@ impl<'a, A: fmt::LowerExp, S, D: Dimension> fmt::LowerExp for ArrayBase<S, D>
where S: Data<Elem=A>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
format_array(self, f, <_>::fmt)
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)
}
}

Expand All @@ -136,7 +265,7 @@ impl<'a, A: fmt::UpperExp, S, D: Dimension> fmt::UpperExp for ArrayBase<S, D>
where S: Data<Elem=A>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
format_array(self, f, <_>::fmt)
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)
}
}
/// Format the array using `LowerHex` and apply the formatting parameters used
Expand All @@ -147,7 +276,7 @@ impl<'a, A: fmt::LowerHex, S, D: Dimension> fmt::LowerHex for ArrayBase<S, D>
where S: Data<Elem=A>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
format_array(self, f, <_>::fmt)
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)
}
}

Expand All @@ -159,6 +288,6 @@ impl<'a, A: fmt::Binary, S, D: Dimension> fmt::Binary for ArrayBase<S, D>
where S: Data<Elem=A>,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
format_array(self, f, <_>::fmt)
format_array_v2(self, f, <_>::fmt, PRINT_ELEMENTS_LIMIT)
}
}
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ mod array_serde;
mod arrayformat;
mod data_traits;

pub use arrayformat::PRINT_ELEMENTS_LIMIT;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


pub use crate::aliases::*;

#[allow(deprecated)]
Expand Down
128 changes: 127 additions & 1 deletion tests/format.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
extern crate ndarray;

use ndarray::prelude::*;
use ndarray::rcarr1;
use ndarray::{rcarr1, PRINT_ELEMENTS_LIMIT};

#[test]
fn formatting()
Expand Down Expand Up @@ -36,6 +36,132 @@ fn formatting()
assert_eq!(s, "[01, ff, fe]");
}

#[cfg(test)]
mod formatting_with_omit {
use super::*;

fn print_output_diff(expected: &str, actual: &str) {
println!("Expected output:\n{}\nActual output:\n{}", expected, actual);
}

#[test]
fn dim_1() {
let overflow: usize = 5;
let a = Array1::from_elem((PRINT_ELEMENTS_LIMIT * 2 + overflow, ), 1);
let mut expected_output = String::from("[");
a.iter()
.take(PRINT_ELEMENTS_LIMIT)
.for_each(|elem| { expected_output.push_str(format!("{}, ", elem).as_str()) });
expected_output.push_str("...");
a.iter()
.skip(PRINT_ELEMENTS_LIMIT + overflow)
.for_each(|elem| { expected_output.push_str(format!(", {}", elem).as_str()) });
expected_output.push(']');
let actual_output = format!("{}", a);

print_output_diff(&expected_output, &actual_output);
assert_eq!(actual_output, expected_output);
}

#[test]
fn dim_2_last_axis_overflow() {
let overflow: usize = 3;
let a = Array2::from_elem((PRINT_ELEMENTS_LIMIT, PRINT_ELEMENTS_LIMIT * 2 + overflow), 1);
let mut expected_output = String::from("[");

for i in 0..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!("[{}", a[(i, 0)]).as_str());
for j in 1..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(", ...");
for j in PRINT_ELEMENTS_LIMIT + overflow..PRINT_ELEMENTS_LIMIT * 2 + overflow {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(if i < PRINT_ELEMENTS_LIMIT - 1 { "],\n " } else { "]" });
}
expected_output.push(']');
let actual_output = format!("{}", a);

print_output_diff(&expected_output, &actual_output);
assert_eq!(actual_output, expected_output);
}

#[test]
fn dim_2_non_last_axis_overflow() {
let overflow: usize = 5;
let a = Array2::from_elem((PRINT_ELEMENTS_LIMIT * 2 + overflow, PRINT_ELEMENTS_LIMIT), 1);
let mut expected_output = String::from("[");

for i in 0..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!("[{}", a[(i, 0)]).as_str());
for j in 1..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str("],\n ");
}
expected_output.push_str("...,\n ");
for i in PRINT_ELEMENTS_LIMIT + overflow..PRINT_ELEMENTS_LIMIT * 2 + overflow {
expected_output.push_str(format!("[{}", a[(i, 0)]).as_str());
for j in 1..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(if i == PRINT_ELEMENTS_LIMIT * 2 + overflow - 1 {
"]"
} else {
"],\n "
});
}
expected_output.push(']');
let actual_output = format!("{}", a);

print_output_diff(&expected_output, &actual_output);
assert_eq!(actual_output, expected_output);
}

#[test]
fn dim_2_multi_directional_overflow() {
let overflow: usize = 5;
let a = Array2::from_elem(
(PRINT_ELEMENTS_LIMIT * 2 + overflow, PRINT_ELEMENTS_LIMIT * 2 + overflow), 1
);
let mut expected_output = String::from("[");

for i in 0..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!("[{}", a[(i, 0)]).as_str());
for j in 1..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(", ...");
for j in PRINT_ELEMENTS_LIMIT + overflow..PRINT_ELEMENTS_LIMIT * 2 + overflow {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str("],\n ");
}
expected_output.push_str("...,\n ");
for i in PRINT_ELEMENTS_LIMIT + overflow..PRINT_ELEMENTS_LIMIT * 2 + overflow {
expected_output.push_str(format!("[{}", a[(i, 0)]).as_str());
for j in 1..PRINT_ELEMENTS_LIMIT {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(", ...");
for j in PRINT_ELEMENTS_LIMIT + overflow..PRINT_ELEMENTS_LIMIT * 2 + overflow {
expected_output.push_str(format!(", {}", a[(i, j)]).as_str());
}
expected_output.push_str(if i == PRINT_ELEMENTS_LIMIT * 2 + overflow - 1 {
"]"
} else {
"],\n "
});
}
expected_output.push(']');
let actual_output = format!("{}", a);

print_output_diff(&expected_output, &actual_output);
assert_eq!(actual_output, expected_output);
}
}

#[test]
fn debug_format() {
let a = Array2::<i32>::zeros((3, 4));
Expand Down