Skip to content

Commit 58fd9dd

Browse files
authored
Merge pull request #827 from vsbogd/separate-debug-and-display
Separate Debug and Display trait implementations for Atom
2 parents b083e09 + 4149bca commit 58fd9dd

File tree

6 files changed

+138
-112
lines changed

6 files changed

+138
-112
lines changed

lib/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ im = "15.1.0"
1313
rand = "0.8.5"
1414
bitset = "0.1.2"
1515
dyn-fmt = "0.4.0"
16+
itertools = "0.13.0"
1617

1718
# pkg_mgmt deps
1819
xxhash-rust = {version="0.8.7", features=["xxh3"], optional=true }

lib/src/atom/mod.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ impl Clone for Box<dyn GroundedAtom> {
784784

785785
/// Atoms are main components of the atomspace. There are four meta-types of
786786
/// atoms: symbol, expression, variable and grounded.
787-
#[derive(Clone)]
787+
#[derive(Debug, Clone)]
788788
pub enum Atom {
789789
/// Symbol represents some idea or concept. Two symbols having
790790
/// the same name are considered equal and representing the same concept.
@@ -1083,12 +1083,6 @@ impl Display for Atom {
10831083
}
10841084
}
10851085

1086-
impl Debug for Atom {
1087-
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
1088-
Display::fmt(self, f)
1089-
}
1090-
}
1091-
10921086
#[cfg(test)]
10931087
mod test {
10941088
#![allow(non_snake_case)]

lib/src/common/assert.rs

Lines changed: 82 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,117 +1,114 @@
11
use super::collections::{ListMap, Equality, DefaultEquality};
22

3-
use std::cmp::Ordering;
3+
use std::fmt::{Debug, Display, Formatter};
4+
use itertools::Itertools;
45

5-
pub fn vec_eq_no_order<'a, T, A, B>(left: A, right: B) -> Option<String>
6-
where
7-
T: 'a + PartialEq + std::fmt::Debug,
8-
A: Iterator<Item=&'a T>,
9-
B: Iterator<Item=&'a T>,
10-
{
11-
compare_vec_no_order(left, right, DefaultEquality{}).as_string()
12-
}
13-
14-
pub fn compare_vec_no_order<'a, T, A, B, E>(left: A, right: B, _cmp: E) -> VecDiff<'a, T, E>
6+
pub fn compare_vec_no_order<'a, T, A, B, E>(actual: A, expected: B, _cmp: E) -> VecDiff<'a, T, E>
157
where
168
A: Iterator<Item=&'a T>,
179
B: Iterator<Item=&'a T>,
1810
E: Equality<&'a T>,
1911
{
20-
let mut left_count: ListMap<&T, usize, E> = ListMap::new();
21-
let mut right_count: ListMap<&T, usize, E> = ListMap::new();
22-
for i in left {
23-
*left_count.entry(&i).or_insert(0) += 1;
12+
let mut diff: ListMap<&T, Count, E> = ListMap::new();
13+
for i in actual {
14+
diff.entry(&i).or_default().actual += 1;
2415
}
25-
for i in right {
26-
*right_count.entry(&i).or_insert(0) += 1;
16+
for i in expected {
17+
diff.entry(&i).or_default().expected += 1;
2718
}
28-
VecDiff{ left_count, right_count }
19+
diff = diff.into_iter().filter(|(_v, c)| c.actual != c.expected).collect();
20+
VecDiff{ diff }
21+
}
22+
23+
#[derive(Default)]
24+
struct Count {
25+
actual: usize,
26+
expected: usize,
2927
}
3028

3129
pub struct VecDiff<'a, T, E: Equality<&'a T>> {
32-
left_count: ListMap<&'a T, usize, E>,
33-
right_count: ListMap<&'a T, usize, E>,
30+
diff: ListMap<&'a T, Count, E>,
31+
}
32+
33+
struct FormatAsDebug<T: Debug>(T);
34+
impl<T: Debug> Display for FormatAsDebug<T> {
35+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
36+
Debug::fmt(&self.0, f)
37+
}
3438
}
3539

36-
trait DiffVisitor<'a, T> {
37-
fn diff(&mut self, item: &'a T, left: usize, right: usize) -> bool;
40+
struct FormatAsDisplay<T: Display>(T);
41+
impl<T: Display> Display for FormatAsDisplay<T> {
42+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
43+
Display::fmt(&self.0, f)
44+
}
3845
}
3946

40-
impl<'a, T: std::fmt::Debug, E: Equality<&'a T>> VecDiff<'a, T, E> {
47+
impl<'a, T, E: Equality<&'a T>> VecDiff<'a, T, E> {
4148
pub fn has_diff(&self) -> bool {
42-
#[derive(Default)]
43-
struct FindDiff {
44-
diff: bool,
45-
}
46-
impl<T: std::fmt::Debug> DiffVisitor<'_, T> for FindDiff {
47-
fn diff(&mut self, _item: &T, left: usize, right: usize) -> bool {
48-
if left == right {
49-
false
50-
} else {
51-
self.diff = true;
52-
true
53-
}
54-
}
55-
}
56-
let mut f = FindDiff::default();
57-
self.visit(&mut f);
58-
f.diff
49+
!self.diff.is_empty()
5950
}
6051

61-
pub fn as_string(&self) -> Option<String> {
62-
#[derive(Default)]
63-
struct StringDiff {
64-
diff: Option<String>,
65-
}
66-
impl<'a, T: std::fmt::Debug> DiffVisitor<'a, T> for StringDiff {
67-
fn diff(&mut self, item: &'a T, left: usize, right: usize) -> bool {
68-
match left.cmp(&right) {
69-
Ordering::Less => {
70-
self.diff = Some(format!("Missed result: {:?}", item));
71-
true
72-
},
73-
Ordering::Greater => {
74-
self.diff = Some(format!("Excessive result: {:?}", item));
75-
true
76-
},
77-
Ordering::Equal => false,
78-
}
79-
}
80-
}
81-
let mut d = StringDiff{ diff: None };
82-
self.visit(&mut d);
83-
d.diff
52+
pub fn as_display(&self) -> Option<String> where T: Display {
53+
self.as_string(FormatAsDisplay)
8454
}
8555

86-
fn visit<'b, V: DiffVisitor<'b, T>>(&'b self, visitor: &mut V) {
87-
for e in self.right_count.iter() {
88-
let count = self.left_count.get(e.0).unwrap_or(&0);
89-
if visitor.diff(e.0, *count, *e.1) { return }
90-
}
91-
for e in self.left_count.iter() {
92-
let count = self.right_count.get(e.0).unwrap_or(&0);
93-
if visitor.diff(e.0, *e.1, *count) { return }
56+
pub fn as_debug(&self) -> Option<String> where T: Debug {
57+
self.as_string(FormatAsDebug)
58+
}
59+
60+
fn as_string<F, I: Display>(&self, f: F) -> Option<String>
61+
where F: Fn(&'a T) -> I
62+
{
63+
let mut diff = String::new();
64+
if self.has_diff() {
65+
let mut missed = self.diff.iter()
66+
.filter(|(_v, c)| c.actual < c.expected)
67+
.flat_map(|(v, c)| std::iter::repeat_n(v, c.expected - c.actual))
68+
.map(|v| f(v))
69+
.peekable();
70+
let mut excessive = self.diff.iter()
71+
.filter(|(_v, c)| c.actual > c.expected)
72+
.flat_map(|(v, c)| std::iter::repeat_n(v, c.actual - c.expected))
73+
.map(|v| f(v))
74+
.peekable();
75+
if missed.peek().is_some() {
76+
diff.push_str(format!("Missed results: {}", missed.format(", ")).as_str());
77+
}
78+
if excessive.peek().is_some() {
79+
if !diff.is_empty() {
80+
diff.push_str("\n");
81+
}
82+
diff.push_str(format!("Excessive results: {}", excessive.format(", ")).as_str());
83+
}
84+
Some(diff)
85+
} else {
86+
None
9487
}
9588
}
9689
}
9790

9891
#[macro_export]
9992
macro_rules! assert_eq_no_order {
100-
($left:expr, $right:expr) => {
93+
($actual:expr, $expected:expr) => {
10194
{
102-
assert!($crate::common::assert::vec_eq_no_order($left.iter(), $right.iter()) == None,
103-
"(left == right some order)\n left: {:?}\n right: {:?}", $left, $right);
95+
let diff = $crate::common::assert::compare_vec_no_order($actual.iter(), $expected.iter(),
96+
$crate::common::collections::DefaultEquality{}).as_debug();
97+
assert!(diff.is_none(),
98+
"(actual != expected)\nActual: {:?}\nExpected: {:?}\n{}",
99+
$actual, $expected, diff.unwrap());
104100
}
105101
}
106102
}
107103

108-
pub fn metta_results_eq<T: PartialEq + std::fmt::Debug>(
109-
left: &Result<Vec<Vec<T>>, String>, right: &Result<Vec<Vec<T>>, String>) -> bool
104+
pub fn metta_results_eq<T: PartialEq>(
105+
actual: &Result<Vec<Vec<T>>, String>, expected: &Result<Vec<Vec<T>>, String>) -> bool
110106
{
111-
match (left, right) {
112-
(Ok(left), Ok(right)) if left.len() == right.len() => {
113-
for (left, right) in left.iter().zip(right.iter()) {
114-
if vec_eq_no_order(left.iter(), right.iter()).is_some() {
107+
match (actual, expected) {
108+
(Ok(actual), Ok(expected)) if actual.len() == expected.len() => {
109+
for (actual, expected) in actual.iter().zip(expected.iter()) {
110+
let diff = compare_vec_no_order(actual.iter(), expected.iter(), DefaultEquality{});
111+
if diff.has_diff() {
115112
return false;
116113
}
117114
}
@@ -123,12 +120,12 @@ pub fn metta_results_eq<T: PartialEq + std::fmt::Debug>(
123120

124121
#[macro_export]
125122
macro_rules! assert_eq_metta_results {
126-
($left:expr, $right:expr) => {
123+
($actual:expr, $expected:expr) => {
127124
{
128-
let left = &$left;
129-
let right = &$right;
130-
assert!($crate::common::assert::metta_results_eq(left, right),
131-
"(left == right)\n left: {:?}\n right: {:?}", left, right);
125+
let actual = &$actual;
126+
let expected = &$expected;
127+
assert!($crate::common::assert::metta_results_eq(actual, expected),
128+
"(actual == expected)\n actual: {:?}\n expected: {:?}", actual, expected);
132129
}
133130
}
134131
}

lib/src/common/collections.rs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::fmt::Display;
2+
use itertools::Itertools;
23

34
pub trait Equality<T> {
45
fn eq(a: &T, b: &T) -> bool;
@@ -26,6 +27,10 @@ pub enum ListMapEntry<'a, K, V, E: Equality<K>> {
2627
}
2728

2829
impl<'a, K, V, E: Equality<K>> ListMapEntry<'a, K, V, E> {
30+
pub fn or_default(self) -> &'a mut V where V: Default {
31+
self.or_insert(Default::default())
32+
}
33+
2934
pub fn or_insert(self, default: V) -> &'a mut V {
3035
match self {
3136
ListMapEntry::Occupied(key, map) => map.get_mut(&key).unwrap(),
@@ -55,6 +60,8 @@ macro_rules! list_map_iterator {
5560
list_map_iterator!(ListMapIter, Iter, {});
5661
list_map_iterator!(ListMapIterMut, IterMut, { mut });
5762

63+
type ListMapIntoIter<K, V> = std::vec::IntoIter<(K, V)>;
64+
5865
macro_rules! list_map_get {
5966
($get:ident, {$( $mut_:tt )?}) => {
6067
pub fn $get(& $( $mut_ )? self, key: &K) -> Option<& $( $mut_ )? V> {
@@ -92,6 +99,10 @@ impl<K, V, E: Equality<K>> ListMap<K, V, E> {
9299
&mut self.list.last_mut().unwrap().1
93100
}
94101

102+
pub fn is_empty(&self) -> bool {
103+
self.list.is_empty()
104+
}
105+
95106
pub fn clear(&mut self) {
96107
self.list.clear()
97108
}
@@ -103,11 +114,15 @@ impl<K, V, E: Equality<K>> ListMap<K, V, E> {
103114
pub fn iter_mut(&mut self) -> ListMapIterMut<'_, K, V> {
104115
ListMapIterMut{ delegate: self.list.iter_mut() }
105116
}
117+
118+
pub fn into_iter(self) -> ListMapIntoIter<K, V> {
119+
self.list.into_iter()
120+
}
106121
}
107122

108-
impl<K: PartialEq, V: PartialEq> PartialEq for ListMap<K, V> {
123+
impl<K, V: PartialEq, E: Equality<K>> PartialEq for ListMap<K, V, E> {
109124
fn eq(&self, other: &Self) -> bool {
110-
fn left_includes_right<K: PartialEq, V: PartialEq>(left: &ListMap<K, V>, right: &ListMap<K, V>) -> bool {
125+
fn left_includes_right<K, V: PartialEq, E: Equality<K>>(left: &ListMap<K, V, E>, right: &ListMap<K, V, E>) -> bool {
111126
for e in right.iter() {
112127
if left.get(e.0) != Some(e.1) {
113128
return false;
@@ -119,7 +134,7 @@ impl<K: PartialEq, V: PartialEq> PartialEq for ListMap<K, V> {
119134
}
120135
}
121136

122-
impl<K: PartialEq, V: PartialEq> From<Vec<(K, V)>> for ListMap<K, V> {
137+
impl<K: PartialEq, V> From<Vec<(K, V)>> for ListMap<K, V> {
123138
fn from(list: Vec<(K, V)>) -> Self {
124139
let mut map = ListMap::new();
125140
for (k, v) in list.into_iter() {
@@ -129,6 +144,14 @@ impl<K: PartialEq, V: PartialEq> From<Vec<(K, V)>> for ListMap<K, V> {
129144
}
130145
}
131146

147+
impl<K, V, E: Equality<K>> FromIterator<(K, V)> for ListMap<K, V, E> {
148+
fn from_iter<T>(iter: T) -> Self where T: IntoIterator<Item = (K, V)> {
149+
let mut map = ListMap::new();
150+
iter.into_iter().for_each(|(k, v)| map.insert(k, v));
151+
map
152+
}
153+
}
154+
132155
#[derive(Debug, Clone)]
133156
pub enum ImmutableString {
134157
Allocated(String),
@@ -266,6 +289,14 @@ impl<'a, T> Into<&'a [T]> for &'a CowArray<T> {
266289
}
267290
}
268291

292+
pub struct VecDisplay<'a, T: 'a + Display>(pub &'a Vec<T>);
293+
294+
impl<'a, T: 'a + Display> Display for VecDisplay<'a, T> {
295+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
296+
write!(f, "[{}]", self.0.iter().format(", "))
297+
}
298+
}
299+
269300

270301
#[cfg(test)]
271302
mod test {

0 commit comments

Comments
 (0)