From cbed5e4b2de995dc76cfca3e402bb9c2cee9ae22 Mon Sep 17 00:00:00 2001 From: EmmanuelDodoo Date: Mon, 7 Oct 2024 21:22:02 +0000 Subject: [PATCH 1/3] feat!: Reworked model Scale - Removed generics from models --- src/models/bar.rs | 147 +++++---------- src/models/common.rs | 382 +++++++++++++++++++++++++++++++++++--- src/models/line.rs | 221 +++++++--------------- src/models/stacked_bar.rs | 175 ++++++----------- src/repr/sheet.rs | 157 +++++++--------- src/repr/sheet/tests.rs | 15 +- 6 files changed, 613 insertions(+), 484 deletions(-) diff --git a/src/models/bar.rs b/src/models/bar.rs index 91b6b92..13a7180 100644 --- a/src/models/bar.rs +++ b/src/models/bar.rs @@ -1,27 +1,22 @@ -use std::{ - collections::HashSet, - fmt::{self, Debug}, - hash::Hash, -}; +use std::fmt::{self, Debug}; use super::{Point, Scale}; -use crate::repr::Data; #[derive(Clone, Debug, PartialEq)] -pub struct Bar { +pub struct Bar { pub label: Option, - pub point: Point, + pub point: Point, } -impl Bar { - pub fn new(label: impl Into, point: impl Into>) -> Self { +impl Bar { + pub fn new(label: impl Into, point: impl Into) -> Self { Self { point: point.into(), label: Some(label.into()), } } - pub fn from_point(point: impl Into>) -> Self { + pub fn from_point(point: impl Into) -> Self { Self { point: point.into(), label: None, @@ -35,36 +30,19 @@ impl Bar { } #[derive(Clone, Debug, PartialEq)] -pub struct BarChart -where - X: Clone + Debug, - Y: Clone + Debug, -{ - pub bars: Vec>, +pub struct BarChart { + pub bars: Vec, pub x_label: Option, pub y_label: Option, - pub x_scale: Scale, - pub y_scale: Scale, + pub x_scale: Scale, + pub y_scale: Scale, } #[allow(dead_code)] -impl BarChart -where - X: Eq + Clone + Hash + PartialOrd + ToString + Debug, - Y: Eq + Clone + Hash + PartialOrd + ToString + Debug, -{ - pub fn new( - bars: Vec>, - x_scale: Scale, - y_scale: Scale, - ) -> Result { - match &x_scale { - Scale::List(scale) => Self::assert_list_scales_x(scale, &bars)?, - }; - - match &y_scale { - Scale::List(scale) => Self::assert_list_scales_y(scale, &bars)?, - }; +impl BarChart { + pub fn new(bars: Vec, x_scale: Scale, y_scale: Scale) -> Result { + Self::assert_x_scale(&x_scale, &bars)?; + Self::assert_y_scale(&y_scale, &bars)?; Ok(Self { x_scale, @@ -75,64 +53,24 @@ where }) } - fn assert_list_scales_y(lst: &[Y], bars: &[Bar]) -> Result<(), BarChartError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - - // Check if all points are on scale. - let mut invalid: Option = None; - let valid = bars.iter().fold(true, |acc, curr| { - if !acc { - return acc; + fn assert_x_scale(scale: &Scale, bars: &[Bar]) -> Result<(), BarChartError> { + for x in bars.iter().map(|bar| &bar.point.x) { + if !scale.contains(x) { + return Err(BarChartError::OutOfRange("X".into(), x.to_string())); } - - if !set.contains(&curr.point.y) { - invalid = Some(curr.point.y.clone()); - false - } else { - true - } - }); - - if valid { - Ok(()) - } else { - Err(BarChartError::OutOfRange( - "Y".into(), - invalid.unwrap().to_string(), - )) } - } - - fn assert_list_scales_x(lst: &[X], bars: &[Bar]) -> Result<(), BarChartError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - let mut invalid: Option = None; - - let valid = bars.iter().fold(true, |acc, curr| { - if !acc { - return acc; - } + Ok(()) + } - if !set.contains(&curr.point.x) { - invalid = Some(curr.point.x.clone()); - false - } else { - true + fn assert_y_scale(scale: &Scale, bars: &[Bar]) -> Result<(), BarChartError> { + for y in bars.iter().map(|bar| &bar.point.y) { + if !scale.contains(y) { + return Err(BarChartError::OutOfRange("Y".into(), y.to_string())); } - }); - - if valid { - Ok(()) - } else { - Err(BarChartError::OutOfRange( - "X".into(), - invalid.unwrap().to_string(), - )) } + + Ok(()) } pub fn x_label(mut self, label: impl Into) -> Self { @@ -169,24 +107,32 @@ impl std::error::Error for BarChartError {} #[cfg(test)] mod barchart_tests { + use super::super::ScaleKind; use super::*; + use crate::repr::Data; - fn create_barchart<'a>() -> BarChart { + fn create_barchart() -> BarChart { let p1 = vec!["one", "two", "three", "four", "five"]; let p2 = [1, 2, 3, 4, 5]; let bars = p2 .into_iter() .zip(p1.into_iter()) - .map(|point| Bar::from_point(point)) + .map(|point| Bar::from_point((Data::Integer(point.0), Data::Text(point.1.to_string())))) .collect(); - let x_scale: Scale = { + let x_scale = { let rng = 0..60; + let rng = rng.into_iter().map(From::from); - Scale::List(rng.collect()) + Scale::new(rng, ScaleKind::Integer) + }; + let y_scale = { + let values = vec!["one", "two", "three", "four", "five"]; + let values = values.into_iter().map(ToOwned::to_owned).map(From::from); + + Scale::new(values, ScaleKind::Text) }; - let y_scale: Scale<&str> = Scale::List(vec!["one", "two", "three", "four", "five"]); match BarChart::new(bars, x_scale, y_scale) { Ok(bar) => bar.x_label("Number").y_label("Language"), @@ -194,24 +140,27 @@ mod barchart_tests { } } - fn out_of_range() -> Result, BarChartError> { + fn out_of_range() -> Result { let xs = [1, 5, 6, 11, 15]; let ys = [4, 5, 6, 7, 8]; let bars = xs .into_iter() .zip(ys.into_iter()) - .map(|point| Bar::from_point(point)) + .map(|point| Bar::from_point((Data::Integer(point.0), Data::Integer(point.1)))) .collect(); - let x_scale: Scale = { + let x_scale = { let rng = -5..11; + let rng = rng.into_iter().map(From::from); - Scale::List(rng.collect()) + Scale::new(rng, ScaleKind::Integer) }; - let y_scale: Scale = { + let y_scale = { let rng = 2..10; - Scale::List(rng.collect()) + let rng = rng.into_iter().map(From::from); + + Scale::new(rng, ScaleKind::Integer) }; BarChart::new(bars, x_scale, y_scale) diff --git a/src/models/common.rs b/src/models/common.rs index 0ce80df..528df11 100644 --- a/src/models/common.rs +++ b/src/models/common.rs @@ -1,5 +1,5 @@ -use crate::repr::Data; -use std::fmt::Debug; +use crate::repr::{ColumnType, Data}; +use std::{collections::HashSet, fmt::Debug}; #[derive(Debug, Clone, PartialEq)] pub struct Point { @@ -19,34 +19,370 @@ impl From<(X, Y)> for Point { } } -#[derive(Debug, Clone, PartialEq)] -pub enum Scale -where - T: Clone + Debug, -{ - // Range(Range), - List(Vec), +/// Determines how points on the scale are handled +/// +/// +/// Points on a [`ScaleKind::Text`] are treated categorically with all duplicates removed and in an arbitary order. Points on other [`ScaleKind`] are treated numerically as a range +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum ScaleKind { + Number, + Integer, + Float, + Text, } -impl Scale -where - T: Clone + Debug, -{ - pub fn points(&self) -> Vec { - match self { - Self::List(lst) => lst.clone(), +impl From for ScaleKind { + fn from(value: ColumnType) -> Self { + match value { + ColumnType::Number => ScaleKind::Number, + ColumnType::Integer => ScaleKind::Integer, + ColumnType::Float => ScaleKind::Float, + _ => ScaleKind::Text, } } } -impl From> for Vec -where - T: Clone + Debug, -{ - fn from(value: Scale) -> Self { - match value { - Scale::List(lst) => lst, +#[derive(Debug, Clone, PartialEq)] +enum ScaleValues { + Number { + start: isize, + end: isize, + step: isize, + }, + Integer { + start: i32, + end: i32, + step: i32, + }, + Float { + start: f32, + end: f32, + step: f32, + }, + Text(Vec), +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Scale { + pub kind: ScaleKind, + values: ScaleValues, +} + +impl Scale { + /// Returns a new scale of the specified type from the given points. + /// If the scale type specified cannot be created from the points, a [`ScaleKind::Text`] is + /// created instead. + pub(crate) fn new(points: impl Iterator, kind: ScaleKind) -> Self { + match kind { + ScaleKind::Text => { + let values = points + .map(|point| point.to_string()) + .collect::>(); + let values = ScaleValues::Text(values.into_iter().collect()); + + Self { kind, values } + } + ScaleKind::Integer => { + let mut valid = HashSet::new(); + let mut invalid = HashSet::new(); + + for point in points { + match point { + Data::Integer(num) => { + valid.insert(num); + } + other => { + invalid.insert(other.to_string()); + } + }; + } + + if !invalid.is_empty() { + for point in valid.into_iter() { + invalid.insert(point.to_string()); + } + + Self { + kind: ScaleKind::Text, + values: ScaleValues::Text(invalid.into_iter().collect()), + } + } else { + Self::from_i32(valid.into_iter().collect::>()) + } + } + ScaleKind::Number => { + let mut valid = HashSet::new(); + let mut invalid = HashSet::new(); + + for point in points { + match point { + Data::Number(num) => { + valid.insert(num); + } + other => { + invalid.insert(other.to_string()); + } + } + } + + if !invalid.is_empty() { + for point in valid.into_iter() { + invalid.insert(point.to_string()); + } + + Self { + kind: ScaleKind::Text, + values: ScaleValues::Text(invalid.into_iter().collect()), + } + } else { + Self::from_isize(valid.into_iter().collect::>()) + } + } + ScaleKind::Float => { + // f32 doesn't implement Hash or Eq + let mut valid: Vec = Vec::new(); + let mut invalid = HashSet::new(); + + for point in points { + match point { + Data::Float(float) => { + if !valid.iter().any(|x| *x == float) { + valid.push(float); + } + } + other => { + invalid.insert(other.to_string()); + } + } + } + + if !invalid.is_empty() { + for point in valid.into_iter() { + invalid.insert(point.to_string()); + } + + Self { + kind: ScaleKind::Text, + values: ScaleValues::Text(invalid.into_iter().collect()), + } + } else { + Self::from_f32(valid) + } + } + } + } + + pub fn points(&self) -> Vec { + match &self.values { + ScaleValues::Text(values) => values.iter().cloned().map(From::from).collect(), + ScaleValues::Number { start, end, step } => { + let mut output = Vec::default(); + let n = ((end - start) / step) + 1; + + for i in 0..n { + let curr = *start + (i * step); + output.push(Data::Number(curr)); + } + + output + } + ScaleValues::Integer { start, end, step } => { + let mut output = Vec::default(); + let n = ((end - start) / step) + 1; + + for i in 0..n { + let curr = *start + (i * step); + output.push(Data::Integer(curr)); + } + + output + } + ScaleValues::Float { start, end, step } => { + let mut output = Vec::default(); + let n = (((end - start) / step) + 1.0) as isize; + + for i in 0..n { + let curr = *start + ((i as f32) * step); + output.push(Data::Float(curr)); + } + + output + } + } + } + + pub fn contains(&self, value: &Data) -> bool { + match (&self.values, value) { + (ScaleValues::Text(values), Data::Text(val)) => values.contains(val), + (ScaleValues::Number { start, end, .. }, Data::Number(num)) => { + start <= num && num <= end + } + (ScaleValues::Integer { start, end, .. }, Data::Integer(num)) => { + start <= num && num <= end + } + (ScaleValues::Float { start, end, .. }, Data::Float(num)) => start <= num && num <= end, + _ => false, + } + } + + /// Assumes points is not empty + pub fn from_i32(points: impl Into>) -> Self { + let mut points: Vec = points.into(); + points.sort(); + + // Scale contains both negative and positive values + let temp = points.iter().fold((false, false), |acc, curr| { + if acc.0 && acc.1 { + acc + } else if *curr < 0 { + (true, acc.1) + } else { + (acc.0, true) + } + }); + + let len = points.len(); + let min = points.first().unwrap(); // Guaranteed by iteration in Self::new + let max = points.get(len - 1).unwrap(); // Guaranteed by iteration in Self::new + let len = if temp.0 && temp.1 { + points.len() + 1 + } else { + points.len() + }; + + let mut step = (max - min) / len as i32; + + if step * (len as i32) != max - min { + step += 1 } + + Self { + kind: ScaleKind::Integer, + values: ScaleValues::Integer { + start: *min, + end: *max, + step, + }, + } + } + + /// Assumes points is not empty + pub fn from_isize(points: impl Into>) -> Self { + let mut points: Vec = points.into(); + points.sort(); + + let temp = points.iter().fold((false, false), |acc, curr| { + if acc.0 && acc.1 { + acc + } else if *curr < 0 { + (true, acc.1) + } else { + (acc.0, true) + } + }); + + let len = points.len(); + let min = points.first().unwrap(); // Guaranteed by iteration in Self::new + let max = points.get(len - 1).unwrap(); // Guaranteed by iteration in Self::new + let len = if temp.0 && temp.1 { + points.len() + 1 + } else { + points.len() + }; + + let mut step = (max - min) / len as isize; + + if step * (len as isize) != max - min { + step += 1 + } + + Self { + kind: ScaleKind::Number, + values: ScaleValues::Number { + start: *min, + end: *max, + step, + }, + } + } + + pub fn from_f32(points: impl Into>) -> Self { + let points: Vec = points.into(); + + let mut min = *points.first().unwrap(); + let mut max = *points.first().unwrap(); + + for point in points.iter() { + let point = *point; + + if point < min { + min = point; + continue; + } + + if point > max { + max = point; + continue; + } + } + + let temp = points.iter().fold((false, false), |acc, curr| { + if acc.0 && acc.1 { + acc + } else if *curr < 0.0 { + (true, acc.1) + } else { + (acc.0, true) + } + }); + + let len = if temp.0 && temp.1 { + points.len() + 1 + } else { + points.len() + }; + + let mut step = (max - min) / len as f32; + + if step * (len as f32) != max - min { + step += 1.0 + } + + Self { + kind: ScaleKind::Float, + values: ScaleValues::Float { + start: min, + end: max, + step, + }, + } + } + + pub fn sort(&mut self) { + if let ScaleValues::Text(values) = &mut self.values { + values.sort(); + } + } +} + +impl From> for Scale { + fn from(value: Vec) -> Self { + let values = value.into_iter().map(From::from); + Self::new(values, ScaleKind::Integer) + } +} + +impl From> for Scale { + fn from(value: Vec) -> Self { + let values = value.into_iter().map(From::from); + Self::new(values, ScaleKind::Number) + } +} + +impl From> for Scale { + fn from(value: Vec) -> Self { + let values = value.into_iter().map(From::from); + Self::new(values, ScaleKind::Float) } } diff --git a/src/models/line.rs b/src/models/line.rs index 5d0f0ba..01f2fda 100644 --- a/src/models/line.rs +++ b/src/models/line.rs @@ -1,25 +1,27 @@ use crate::repr::Data; -use std::{collections::HashSet, fmt::Debug, hash::Hash, ops::Range}; +use std::fmt::Debug; pub use utils::*; use super::{Point, Scale}; #[derive(Debug, Clone, PartialEq)] -pub struct Line { - pub points: Vec>, +pub struct Line { + pub points: Vec>, pub label: Option, } -impl Line { - pub fn new(points: Vec<(X, Y)>) -> Self { - let points = points.into_iter().map(|(x, y)| Point::new(x, y)); +impl Line { + pub fn new(points: impl IntoIterator, impl Into)>) -> Self { + let points = points + .into_iter() + .map(|(x, y)| Point::new(x.into(), y.into())); Self { points: points.collect(), label: None, } } - pub fn from_points(points: impl IntoIterator>) -> Self { + pub fn from_points(points: impl IntoIterator>) -> Self { Self { points: points.into_iter().collect(), label: None, @@ -33,44 +35,30 @@ impl Line { } #[derive(Debug, Clone, PartialEq)] -pub struct LineGraph -where - X: Clone + Debug, - Y: Clone + Debug, -{ - pub lines: Vec>, +pub struct LineGraph { + pub lines: Vec, pub x_label: String, pub y_label: String, - pub x_scale: Scale, - pub y_scale: Scale, + pub x_scale: Scale, + pub y_scale: Scale, } #[allow(dead_code)] -impl LineGraph -where - X: Eq + Clone + Hash + PartialOrd + ToString + Debug, - Y: Eq + Clone + Hash + PartialOrd + ToString + Debug, -{ +impl LineGraph { pub fn new( - lines: Vec>, + lines: Vec, x_label: Option, y_label: Option, - x_scale: Scale, - y_scale: Scale, + x_scale: Scale, + y_scale: Scale, ) -> Result { let x_label = x_label.unwrap_or_default(); let y_label = y_label.unwrap_or_default(); - match &x_scale { - // Scale::Range(rng) => Scale::Range(LineGraph::assert_range_scales_x(rng, &lines)?), - Scale::List(lst) => LineGraph::assert_list_scales_x(lst, &lines)?, - }; + LineGraph::assert_x_scale(&x_scale, &lines)?; - match &y_scale { - // Scale::Range(rng) => Scale::Range(LineGraph::assert_range_scales_y(rng, &lines)?), - Scale::List(lst) => LineGraph::assert_list_scales_y(lst, &lines)?, - }; + LineGraph::assert_y_scale(&y_scale, &lines)?; Ok(Self { lines, @@ -81,113 +69,30 @@ where }) } - fn assert_range_scales_x( - rng: Range, - lines: &[Line], - ) -> Result, LineGraphError> { - let rng = rng.start..rng.end; - - let mut invalid: Option = None; - - let valid = lines.iter().fold(true, |acc, curr| { - return acc - && curr.points.iter().fold(true, |acc, curr| { - if !rng.contains(&curr.x) { - invalid = Some(curr.x.clone()); - } - acc && rng.contains(&curr.x) - }); - }); - - if valid { - Ok(rng) - } else { - Err(LineGraphError::OutOfRange( - "X".into(), - invalid.unwrap().to_string(), - )) + fn assert_x_scale(scale: &Scale, lines: &[Line]) -> Result<(), LineGraphError> { + for x in lines + .iter() + .flat_map(|line| line.points.iter().map(|point| &point.x)) + { + if !scale.contains(x) { + return Err(LineGraphError::OutOfRange("X".into(), x.to_string())); + } } - } - - fn assert_range_scales_y( - rng: Range, - lines: &[Line], - ) -> Result, LineGraphError> { - let rng = rng.start..rng.end; - let mut invalid: Option = None; - let valid = lines.iter().fold(true, |acc, curr| { - return acc - && curr.points.iter().fold(true, |acc, curr| { - if !rng.contains(&curr.y) { - invalid = Some(curr.y.clone()); - } - acc && rng.contains(&curr.y) - }); - }); - if valid { - Ok(rng) - } else { - Err(LineGraphError::OutOfRange( - "Y".into(), - invalid.unwrap().to_string(), - )) - } + Ok(()) } - fn assert_list_scales_x(lst: &[X], lines: &[Line]) -> Result<(), LineGraphError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - - let mut invalid: Option = None; - - // Check if all points are on scale. - let valid = lines.iter().fold(true, |acc, cur| { - return acc - && cur.points.iter().fold(true, |acc, curr| { - if !set.contains(&curr.x) { - invalid = Some(curr.x.clone()); - } - acc && set.contains(&curr.x) - }); - }); - - if valid { - Ok(()) - } else { - Err(LineGraphError::OutOfRange( - "X".into(), - invalid.unwrap().to_string(), - )) + fn assert_y_scale(scale: &Scale, lines: &[Line]) -> Result<(), LineGraphError> { + for y in lines + .iter() + .flat_map(|line| line.points.iter().map(|point| &point.y)) + { + if !scale.contains(y) { + return Err(LineGraphError::OutOfRange("Y".into(), y.to_string())); + } } - } - fn assert_list_scales_y(lst: &[Y], lines: &[Line]) -> Result<(), LineGraphError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - - // Check if all points are on scale. - let mut invalid: Option = None; - let valid = lines.iter().fold(true, |acc, cur| { - return acc - && cur.points.iter().fold(true, |acc, curr| { - if !set.contains(&curr.y) { - invalid = Some(curr.y.clone()) - } - acc && set.contains(&curr.y) - }); - }); - - if valid { - Ok(()) - } else { - Err(LineGraphError::OutOfRange( - "Y".into(), - invalid.unwrap().to_string(), - )) - } + Ok(()) } } @@ -222,27 +127,31 @@ pub mod utils { #[cfg(test)] mod line_tests { + use super::super::common::ScaleKind; use super::*; fn create_point(x: X, y: Y) -> Point { Point::new(x, y) } - fn create_line_from_points(xs: Vec<&str>, label: impl Into) -> Line { - let points: Vec> = xs + fn create_line_from_points(xs: Vec<&str>, label: impl Into) -> Line { + let points: Vec = xs .into_iter() .enumerate() - .map(|(i, x)| create_point(i, x)) + .map(|(i, x)| create_point(Data::Number(i as isize), Data::Text(x.to_owned()))) .collect(); Line::from_points(points).label(label) } - fn create_line_from_new(xs: Vec<(usize, &str)>, label: impl Into) -> Line { - Line::new(xs).label(label) + fn create_line_from_new(xs: Vec<(usize, &str)>, label: impl Into) -> Line { + let points = xs + .iter() + .map(|(idx, text)| (Data::Number(*idx as isize), Data::Text(text.to_string()))); + Line::new(points).label(label) } - fn create_graph<'a>() -> LineGraph { + fn create_graph() -> LineGraph { let p1 = vec!["one", "two", "three", "four", "five"]; let p2: Vec<(usize, &str)> = vec![ (10, "one"), @@ -255,13 +164,19 @@ mod line_tests { let pnt1 = create_line_from_new(p2, "Deutsch"); let pnt2 = create_line_from_points(p1, "English"); - // let x_scale: Scale = Scale::Range(0..60); - let x_scale: Scale = { + let x_scale = { let rng = 0..60; + let rng = rng.into_iter().map(|num| Data::Number(num as isize)); + + Scale::new(rng, ScaleKind::Number) + }; - Scale::List(rng.collect()) + let y_scale = { + let values = vec!["one", "two", "three", "four", "five"]; + let values = values.into_iter().map(ToOwned::to_owned).map(From::from); + + Scale::new(values, ScaleKind::Text) }; - let y_scale: Scale<&str> = Scale::List(vec!["one", "two", "three", "four", "five"]); match LineGraph::new( vec![pnt1, pnt2], @@ -275,18 +190,21 @@ mod line_tests { } } - fn faulty_graph1() -> Result, LineGraphError> { - let p1: Vec<(isize, isize)> = vec![(0, 0), (1, 1), (20, 2), (3, 35)]; - let p2: Vec<(isize, isize)> = vec![(10, 10), (4, 8), (-3, 3)]; + fn faulty_graph1() -> Result { + let p1: Vec<(i32, i32)> = vec![(0, 0), (1, 1), (20, 2), (3, 35)]; + let p2: Vec<(i32, i32)> = vec![(10, 10), (4, 8), (-3, 3)]; - let x_scale: Scale = { + let x_scale: Scale = { let rng = -5..11; + let rng = rng.into_iter().map(From::from); - Scale::List(rng.collect()) + Scale::new(rng, ScaleKind::Integer) }; - let y_scale: Scale = { + let y_scale: Scale = { let rng = 2..10; - Scale::List(rng.collect()) + let rng = rng.into_iter().map(From::from); + + Scale::new(rng, ScaleKind::Integer) }; let l1 = Line::new(p1); @@ -301,9 +219,10 @@ mod line_tests { let line = create_line_from_points(pts, "Line 1"); assert_eq!(line.label, Some(String::from("Line 1"))); - let temp: Vec<&str> = line.points.iter().fold(vec![], |acc, curr| { + + let temp: Vec = line.points.iter().fold(vec![], |acc, curr| { let mut acc = acc.clone(); - acc.push(curr.y); + acc.push(curr.y.to_string()); acc }); assert_eq!(vec!["one", "two", "three"], temp) diff --git a/src/models/stacked_bar.rs b/src/models/stacked_bar.rs index 83c59c9..8c28a09 100644 --- a/src/models/stacked_bar.rs +++ b/src/models/stacked_bar.rs @@ -1,36 +1,28 @@ use std::{ collections::{HashMap, HashSet}, fmt::{self, Debug}, - hash::Hash, }; use super::{Point, Scale}; use crate::repr::Data; #[derive(Clone, Debug, PartialEq)] -pub struct StackedBar { +pub struct StackedBar { /// The (x, y) points for the bar - pub point: Point, + pub point: Point, /// The percentage makeup of the bar. For all /// k, v in `fractions` v1 + v2 + v3 + .. = 1.0 pub fractions: HashMap, /// Is true of all points within the bar are negative pub is_negative: bool, /// The full value of the stacked bar - true_y: Y, + true_y: Data, /// Keeps track of sections removed from the bar removed_sections: HashSet, } -impl StackedBar -where - Y: Clone, -{ - pub(crate) fn new( - point: Point, - fractions: HashMap, - is_negative: bool, - ) -> Self { +impl StackedBar { + pub(crate) fn new(point: Point, fractions: HashMap, is_negative: bool) -> Self { let true_y = point.y.clone(); Self { point, @@ -41,7 +33,7 @@ where } } - pub fn from_point(point: impl Into>, is_negative: bool) -> Self { + pub fn from_point(point: impl Into, is_negative: bool) -> Self { let point = point.into(); let true_y = point.y.clone(); Self { @@ -61,12 +53,10 @@ where &self.fractions } - pub fn get_point(&self) -> &Point { + pub fn get_point(&self) -> &Point { &self.point } -} -impl StackedBar { /// Returns true if the point is empty. For a Stacked bar chart, an empty point /// is defined as one which has a y data value of 0 or 0.0 pub(crate) fn is_empty(&self) -> bool { @@ -140,40 +130,27 @@ impl StackedBar { } #[derive(Clone, Debug, PartialEq)] -pub struct StackedBarChart -where - X: Clone + Debug, - Y: Clone + Debug, -{ - pub bars: Vec>, +pub struct StackedBarChart { + pub bars: Vec, pub x_axis: Option, pub y_axis: Option, pub labels: HashSet, - pub x_scale: Scale, - pub y_scale: Scale, + pub x_scale: Scale, + pub y_scale: Scale, pub has_negatives: bool, pub has_positives: bool, } #[allow(dead_code)] -impl StackedBarChart -where - X: Eq + Clone + Hash + PartialOrd + ToString + Debug, - Y: Eq + Clone + Hash + PartialOrd + ToString + Debug, -{ +impl StackedBarChart { pub(crate) fn new( - bars: Vec>, - x_scale: Scale, - y_scale: Scale, + bars: Vec, + x_scale: Scale, + y_scale: Scale, labels: HashSet, ) -> Result { - match &x_scale { - Scale::List(scale) => Self::assert_list_scales_x(scale, &bars)?, - }; - - match &y_scale { - Scale::List(scale) => Self::assert_list_scales_y(scale, &bars)?, - }; + Self::assert_x_scale(&x_scale, &bars)?; + Self::assert_y_scale(&y_scale, &bars)?; let has_negatives = bars.iter().any(|bar| bar.is_negative); @@ -190,70 +167,30 @@ where }) } - fn assert_list_scales_y( - lst: &[Y], - bars: &[StackedBar], - ) -> Result<(), StackedBarChartError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - - // Check if all points are on scale. - let mut invalid: Option = None; - let valid = bars.iter().fold(true, |acc, curr| { - if !acc { - return acc; + fn assert_x_scale(scale: &Scale, bars: &[StackedBar]) -> Result<(), StackedBarChartError> { + for x in bars.iter().map(|bar| &bar.point.x) { + if !scale.contains(x) { + return Err(StackedBarChartError::OutOfRange( + "X".to_string(), + x.to_string(), + )); } - - if !set.contains(&curr.point.y) { - invalid = Some(curr.point.y.clone()); - false - } else { - true - } - }); - - if valid { - Ok(()) - } else { - Err(StackedBarChartError::OutOfRange( - "Y".into(), - invalid.unwrap().to_string(), - )) } - } - fn assert_list_scales_x( - lst: &[X], - bars: &[StackedBar], - ) -> Result<(), StackedBarChartError> { - // Duplicate check and removal - let mut lst: Vec = lst.to_vec(); - let set: HashSet = lst.drain(..).collect(); - - let mut invalid: Option = None; - - let valid = bars.iter().fold(true, |acc, curr| { - if !acc { - return acc; - } + Ok(()) + } - if !set.contains(&curr.point.x) { - invalid = Some(curr.point.x.clone()); - false - } else { - true + fn assert_y_scale(scale: &Scale, bars: &[StackedBar]) -> Result<(), StackedBarChartError> { + for y in bars.iter().map(|bar| &bar.point.y) { + if !scale.contains(y) { + return Err(StackedBarChartError::OutOfRange( + "Y".to_string(), + y.to_string(), + )); } - }); - - if valid { - Ok(()) - } else { - Err(StackedBarChartError::OutOfRange( - "X".into(), - invalid.unwrap().to_string(), - )) } + + Ok(()) } pub fn x_axis(mut self, label: impl Into) -> Self { @@ -275,9 +212,7 @@ where self.bars.retain(|bar| bar.is_negative); self.has_positives = false; } -} -impl StackedBarChart { /// Returns true any negative bar is not completely empty. For a Stacked bar chart, an empty point /// is defined as one which has a y data value of 0 or 0.0 pub fn has_true_negatives(&self) -> bool { @@ -344,12 +279,14 @@ impl std::error::Error for StackedBarChartError {} #[cfg(test)] mod stacked_barchart_tests { + use crate::models::ScaleKind; + use super::*; - fn create_barchart<'a>() -> StackedBarChart<&'a str, i32> { + fn create_barchart<'a>() -> StackedBarChart { let mut bars = Vec::with_capacity(5); - let pnt = Point::new("One", 19); + let pnt = Point::new(Data::Text("One".into()), Data::Integer(19)); let fractions = HashMap::from([ (String::from("Soda"), 3.0 / 19.0), @@ -362,7 +299,7 @@ mod stacked_barchart_tests { bars.push(bar); - let pnt = Point::new("Two", 19); + let pnt = Point::new(Data::Text("Two".into()), Data::Integer(19)); let fractions = HashMap::from([ (String::from("Soda"), 3.0 / 19.0), @@ -374,7 +311,7 @@ mod stacked_barchart_tests { let bar = StackedBar::new(pnt, fractions, false); bars.push(bar); - let pnt = Point::new("Three", 14); + let pnt = Point::new(Data::Text("Three".into()), Data::Integer(14)); let fractions = HashMap::from([ (String::from("Soda"), 6.0 / 14.0), @@ -386,7 +323,7 @@ mod stacked_barchart_tests { let bar = StackedBar::new(pnt, fractions, false); bars.push(bar); - let pnt = Point::new("Four", 16); + let pnt = Point::new(Data::Text("Four".into()), Data::Integer(16)); let fractions = HashMap::from([ (String::from("Soda"), 3.0 / 16.0), @@ -398,7 +335,7 @@ mod stacked_barchart_tests { let bar = StackedBar::new(pnt, fractions, false); bars.push(bar); - let pnt = Point::new("Five", 19); + let pnt = Point::new(Data::Text("Five".into()), Data::Integer(19)); let fractions = HashMap::from([ (String::from("Soda"), 9.0 / 19.0), @@ -410,13 +347,14 @@ mod stacked_barchart_tests { let bar = StackedBar::new(pnt, fractions, false); bars.push(bar); - let x_scale: Scale<&str> = { - let lst = vec!["One", "Two", "Three", "Four", "Five"]; + let x_scale = { + let values = vec!["One", "Two", "Three", "Four", "Five"]; + let values = values.into_iter().map(ToOwned::to_owned).map(From::from); - Scale::List(lst) + Scale::new(values, ScaleKind::Text) }; - let y_scale: Scale = Scale::List(vec![14, 16, 19]); + let y_scale = vec![14, 16, 19].into(); let labels = HashSet::from([ (String::from("Soda")), @@ -431,24 +369,29 @@ mod stacked_barchart_tests { } } - fn out_of_range() -> Result, StackedBarChartError> { + fn out_of_range() -> Result { let xs = [1, 5, 6, 11, 15]; let ys = [4, 5, 6, 7, 8]; let bars = xs .into_iter() .zip(ys.into_iter()) - .map(|point| StackedBar::from_point(point, false)) + .map(|point| { + StackedBar::from_point((Data::Integer(point.0), Data::Integer(point.1)), false) + }) .collect(); - let x_scale: Scale = { + let x_scale = { let rng = -5..11; + let rng = rng.into_iter().map(From::from); - Scale::List(rng.collect()) + Scale::new(rng, ScaleKind::Integer) }; - let y_scale: Scale = { + let y_scale = { let rng = 2..10; - Scale::List(rng.collect()) + let rng = rng.into_iter().map(From::from); + + Scale::new(rng, ScaleKind::Integer) }; StackedBarChart::new(bars, x_scale, y_scale, HashSet::default()) diff --git a/src/repr/sheet.rs b/src/repr/sheet.rs index b427f9e..2cb49e3 100644 --- a/src/repr/sheet.rs +++ b/src/repr/sheet.rs @@ -10,6 +10,7 @@ use crate::models::{ bar::{Bar, BarChart}, line::{Line, LineGraph}, stacked_bar::{StackedBar, StackedBarChart}, + ScaleKind, }; use crate::models::{Point, Scale}; @@ -209,11 +210,11 @@ impl Row { fn create_line( &self, label: &LineLabelStrategy, - x_values: &[String], + x_values: &[Data], exclude: &HashSet, idx: usize, - ) -> Line { - let points: Vec> = match label { + ) -> Line { + let points: Vec = match label { LineLabelStrategy::FromCell(idx) => { let points = x_values .iter() @@ -835,7 +836,7 @@ impl Sheet { } } - fn validate_to_line_graph(&self, label_strat: &LineLabelStrategy) -> Result<()> { + fn validate_to_line_graph(&self, label_strat: &LineLabelStrategy) -> Result { // None type Columns self.headers.iter().try_fold((), |_acc, curr| { if let ColumnHeader { @@ -865,7 +866,7 @@ impl Sheet { }; // Uniform type columns - match label_strat { + let kind = match label_strat { LineLabelStrategy::FromCell(idx) => { if idx >= &self.headers.len() { return Err(Error::ConversionError( @@ -880,18 +881,17 @@ impl Sheet { .filter(|(ind, _)| ind != idx) .try_fold(ColumnType::None, |acc, (_, ct)| { check_uniform_type(acc, *ct) - })?; + })? } - _ => { - self.headers - .iter() - .map(|hdr| &hdr.kind) - .try_fold(ColumnType::None, |acc, ct| check_uniform_type(acc, *ct))?; - } - } + _ => self + .headers + .iter() + .map(|hdr| &hdr.kind) + .try_fold(ColumnType::None, |acc, ct| check_uniform_type(acc, *ct))?, + }; - Ok(()) + Ok(kind.into()) } fn validate_to_barchart( @@ -899,7 +899,7 @@ impl Sheet { x_col: usize, y_col: usize, bar_label: &BarChartBarLabels, - ) -> Result<()> { + ) -> Result<(ScaleKind, ScaleKind)> { if let BarChartBarLabels::FromColumn(idx) = bar_label { if idx >= &self.headers.len() { return Err(Error::ConversionError( @@ -936,10 +936,14 @@ impl Sheet { )); } - Ok(()) + Ok((x_type.into(), y_type.into())) } - fn validate_to_stacked_bar_chart(&self, x_col: usize, cols: &[usize]) -> Result> { + fn validate_to_stacked_bar_chart( + &self, + x_col: usize, + cols: &[usize], + ) -> Result<(Vec, ScaleKind)> { self.headers.get(x_col).ok_or(Error::ConversionError( "Stacked Bar chart: x column out of range".into(), ))?; @@ -972,7 +976,7 @@ impl Sheet { match kind { Some(ColumnType::Number) | Some(ColumnType::Float) | Some(ColumnType::Integer) => { - Ok(labels) + Ok((labels, kind.unwrap().into())) } Some(_) => Err(Error::ConversionError( "Stacked Bar Chart Cannot accumulate column type".into(), @@ -995,9 +999,9 @@ impl Sheet { label_strat: LineLabelStrategy, exclude_row: HashSet, exclude_column: HashSet, - ) -> Result> { + ) -> Result { self.validate()?; - self.validate_to_line_graph(&label_strat)?; + let scale_kind = self.validate_to_line_graph(&label_strat)?; if self.is_empty() { return Err(Error::ConversionError( @@ -1005,12 +1009,13 @@ impl Sheet { )); } - let x_values: Vec = { - let values: Vec = self.headers.iter().map(|hdr| hdr.label.clone()).collect(); - values - }; + let x_values: Vec = self + .headers + .iter() + .map(|hdr| Data::Text(hdr.label.clone())) + .collect(); - let lines: Vec> = self + let lines: Vec = self .iter_rows() .enumerate() .filter(|(idx, _)| !exclude_row.contains(idx)) @@ -1019,38 +1024,35 @@ impl Sheet { .map(|(idx, rw)| rw.create_line(&label_strat, &x_values, &exclude_column, idx)) .collect(); - let y_scale: Scale = { - let values: HashSet = lines + let y_scale = { + let values = lines .iter() - .flat_map(|ln| ln.points.iter().map(|pnt| pnt.y.clone())) - .collect(); - - let mut values = values.into_iter().collect::>(); - values.sort(); + .flat_map(|ln| ln.points.iter().map(|pnt| pnt.y.clone())); - Scale::List(values) + Scale::new(values, scale_kind) }; - let x_scale = { - let values: HashSet = match label_strat { - LineLabelStrategy::FromCell(ref id) => x_values - .into_iter() - .enumerate() - .filter(|(idx, _)| idx != id && !exclude_column.contains(idx)) - .map(|(_, lbl)| lbl) - .collect(), - _ => x_values - .into_iter() - .enumerate() - .filter(|(idx, _)| !exclude_column.contains(idx)) - .map(|(_, lbl)| lbl) - .collect(), - }; - - let mut values = values.into_iter().collect::>(); - values.sort(); - - Scale::List(values) + let x_scale = match label_strat { + LineLabelStrategy::FromCell(id) => { + let values = x_values.into_iter().enumerate().filter_map(|(idx, lbl)| { + if idx != id && !exclude_column.contains(&idx) { + Some(lbl) + } else { + None + } + }); + Scale::new(values, ScaleKind::Text) + } + _ => { + let values = x_values.into_iter().enumerate().filter_map(|(idx, lbl)| { + if !exclude_column.contains(&idx) { + Some(lbl) + } else { + None + } + }); + Scale::new(values, ScaleKind::Text) + } }; let lg = LineGraph::new(lines, x_label, y_label, x_scale, y_scale) @@ -1066,8 +1068,8 @@ impl Sheet { bar_label: BarChartBarLabels, axis_labels: BarChartAxisLabelStrategy, exclude_row: HashSet, - ) -> Result> { - self.validate_to_barchart(x_col, y_col, &bar_label)?; + ) -> Result { + let (x_kind, y_kind) = self.validate_to_barchart(x_col, y_col, &bar_label)?; if self.is_empty() { return Err(Error::ConversionError( @@ -1138,30 +1140,18 @@ impl Sheet { Some(label) => Bar::new(label, point), None => Bar::from_point(point), }) - .collect::>>(); + .collect::>(); let x_scale = { - let values = bars - .iter() - .map(|bar| bar.point.x.clone()) - .collect::>(); + let values = bars.iter().map(|bar| bar.point.x.clone()); - let mut values = values.into_iter().collect::>(); - values.sort(); - - Scale::List(values) + Scale::new(values, x_kind) }; let y_scale = { - let values = bars - .iter() - .map(|bar| bar.point.y.clone()) - .collect::>(); - - let mut values = values.into_iter().collect::>(); - values.sort(); + let values = bars.iter().map(|bar| bar.point.y.clone()); - Scale::List(values) + Scale::new(values, y_kind) }; let barchart = BarChart::new(bars, x_scale, y_scale)?; @@ -1195,7 +1185,7 @@ impl Sheet { axis_labels: StackedBarChartAxisLabelStrategy, ) -> Result { let cols = cols.into(); - let acc_labels = self.validate_to_stacked_bar_chart(x_col, &cols)?; + let (acc_labels, y_kind) = self.validate_to_stacked_bar_chart(x_col, &cols)?; if self.is_empty() { return Err(Error::ConversionError( @@ -1231,23 +1221,16 @@ impl Sheet { } let x_scale = { - let mut values = x_values - .collect::>() - .into_iter() - .collect::>(); - values.sort(); + let kind = self + .headers + .get(x_col) + .expect("Stacked Bar Chart conversion: Validations failed") + .kind; - Scale::List(values) + Scale::new(x_values, kind.into()) }; - let y_scale = { - let values = y_values.into_iter().collect::>(); - - let mut values = values.into_iter().collect::>(); - values.sort(); - - Scale::List(values) - }; + let y_scale = Scale::new(y_values.into_iter(), y_kind); let acc_labels = acc_labels.into_iter().collect(); diff --git a/src/repr/sheet/tests.rs b/src/repr/sheet/tests.rs index a9a3a05..380bfcb 100644 --- a/src/repr/sheet/tests.rs +++ b/src/repr/sheet/tests.rs @@ -519,7 +519,7 @@ fn test_line_scales() { .build() .expect("Building alter csv failure"); - let line = sht + let mut line = sht .create_line_graph( None, None, @@ -531,10 +531,12 @@ fn test_line_scales() { let expected_x_scale = { let values = vec![1958, 1959, 1960]; + let values = values.into_iter().map(|year| Data::Text(year.to_string())); - Scale::List(values.into_iter().map(|year| year.to_string()).collect()) + Scale::new(values, crate::models::ScaleKind::Integer) }; + line.x_scale.sort(); assert_eq!(line.x_scale, expected_x_scale); let expected_y_scale = { @@ -542,12 +544,9 @@ fn test_line_scales() { 318, 340, 342, 348, 360, 362, 363, 391, 396, 406, 417, 419, 420, 461, 472, ]; - Scale::List( - values - .into_iter() - .map(|value| Into::::into(value)) - .collect(), - ) + let values = values.into_iter().map(|year| Data::Integer(year)); + + Scale::new(values, crate::models::ScaleKind::Integer) }; assert_eq!(line.y_scale, expected_y_scale); From 470d0d99825b64dc1e8e0d4c856dd8ef941d58cc Mon Sep 17 00:00:00 2001 From: EmmanuelDodoo Date: Mon, 7 Oct 2024 22:10:49 +0000 Subject: [PATCH 2/3] feat!: Scale improvements - Added dedup of scale points - Changed visibility of some Scale functions - Added length of scale - Corrected line scales test - Changed Scale::contains to consider entire generated range - Changed Scale::new to accept friendlier inputs - Changed Scale::points to return uniform Data type - Added Scale tests --- src/models/bar.rs | 4 - src/models/common.rs | 466 ++++++++++++++++++++++++++++++-------- src/models/line.rs | 3 - src/models/stacked_bar.rs | 3 - src/repr/sheet/tests.rs | 3 +- 5 files changed, 373 insertions(+), 106 deletions(-) diff --git a/src/models/bar.rs b/src/models/bar.rs index 13a7180..b094806 100644 --- a/src/models/bar.rs +++ b/src/models/bar.rs @@ -123,13 +123,11 @@ mod barchart_tests { let x_scale = { let rng = 0..60; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; let y_scale = { let values = vec!["one", "two", "three", "four", "five"]; - let values = values.into_iter().map(ToOwned::to_owned).map(From::from); Scale::new(values, ScaleKind::Text) }; @@ -152,13 +150,11 @@ mod barchart_tests { let x_scale = { let rng = -5..11; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; let y_scale = { let rng = 2..10; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; diff --git a/src/models/common.rs b/src/models/common.rs index 528df11..faaef94 100644 --- a/src/models/common.rs +++ b/src/models/common.rs @@ -1,5 +1,5 @@ use crate::repr::{ColumnType, Data}; -use std::{collections::HashSet, fmt::Debug}; +use std::{collections::HashSet, fmt::Debug, isize}; #[derive(Debug, Clone, PartialEq)] pub struct Point { @@ -66,21 +66,29 @@ enum ScaleValues { pub struct Scale { pub kind: ScaleKind, values: ScaleValues, + pub length: usize, } impl Scale { /// Returns a new scale of the specified type from the given points. /// If the scale type specified cannot be created from the points, a [`ScaleKind::Text`] is /// created instead. - pub(crate) fn new(points: impl Iterator, kind: ScaleKind) -> Self { + pub(crate) fn new(points: impl IntoIterator>, kind: ScaleKind) -> Self { + let points = points.into_iter().map(Into::into); match kind { ScaleKind::Text => { let values = points .map(|point| point.to_string()) .collect::>(); - let values = ScaleValues::Text(values.into_iter().collect()); - - Self { kind, values } + let values = values.into_iter().collect::>(); + let length = values.len(); + let values = ScaleValues::Text(values); + + Self { + kind, + values, + length, + } } ScaleKind::Integer => { let mut valid = HashSet::new(); @@ -97,17 +105,31 @@ impl Scale { }; } - if !invalid.is_empty() { + if valid.is_empty() && invalid.is_empty() { + Self { + kind, + values: ScaleValues::Integer { + start: 0, + end: 0, + step: 0, + }, + length: 1, + } + } else if !invalid.is_empty() { for point in valid.into_iter() { invalid.insert(point.to_string()); } + let invalid = invalid.into_iter().collect::>(); + let length = invalid.len(); + Self { kind: ScaleKind::Text, - values: ScaleValues::Text(invalid.into_iter().collect()), + values: ScaleValues::Text(invalid), + length, } } else { - Self::from_i32(valid.into_iter().collect::>()) + Self::from_i32(valid.into_iter()) } } ScaleKind::Number => { @@ -125,17 +147,31 @@ impl Scale { } } - if !invalid.is_empty() { + if valid.is_empty() && invalid.is_empty() { + Self { + kind, + values: ScaleValues::Number { + start: 0, + end: 0, + step: 0, + }, + length: 1, + } + } else if !invalid.is_empty() { for point in valid.into_iter() { invalid.insert(point.to_string()); } + let invalid = invalid.into_iter().collect::>(); + let length = invalid.len(); + Self { kind: ScaleKind::Text, - values: ScaleValues::Text(invalid.into_iter().collect()), + values: ScaleValues::Text(invalid), + length, } } else { - Self::from_isize(valid.into_iter().collect::>()) + Self::from_isize(valid.into_iter()) } } ScaleKind::Float => { @@ -156,17 +192,31 @@ impl Scale { } } - if !invalid.is_empty() { + if valid.is_empty() && invalid.is_empty() { + Self { + kind, + values: ScaleValues::Float { + start: 0.0, + end: 0.0, + step: 0.0, + }, + length: 1, + } + } else if !invalid.is_empty() { for point in valid.into_iter() { invalid.insert(point.to_string()); } + let invalid = invalid.into_iter().collect::>(); + let length = invalid.len(); + Self { kind: ScaleKind::Text, - values: ScaleValues::Text(invalid.into_iter().collect()), + values: ScaleValues::Text(invalid), + length, } } else { - Self::from_f32(valid) + Self::from_f32(valid.into_iter()) } } } @@ -174,10 +224,14 @@ impl Scale { pub fn points(&self) -> Vec { match &self.values { - ScaleValues::Text(values) => values.iter().cloned().map(From::from).collect(), - ScaleValues::Number { start, end, step } => { + ScaleValues::Text(values) => values + .iter() + .cloned() + .map(|value| Data::Text(value)) + .collect(), + ScaleValues::Number { start, step, .. } => { let mut output = Vec::default(); - let n = ((end - start) / step) + 1; + let n = self.length as isize; for i in 0..n { let curr = *start + (i * step); @@ -186,9 +240,9 @@ impl Scale { output } - ScaleValues::Integer { start, end, step } => { + ScaleValues::Integer { start, step, .. } => { let mut output = Vec::default(); - let n = ((end - start) / step) + 1; + let n = self.length as i32; for i in 0..n { let curr = *start + (i * step); @@ -197,9 +251,9 @@ impl Scale { output } - ScaleValues::Float { start, end, step } => { + ScaleValues::Float { start, step, .. } => { let mut output = Vec::default(); - let n = (((end - start) / step) + 1.0) as isize; + let n = self.length as isize; for i in 0..n { let curr = *start + ((i as f32) * step); @@ -214,132 +268,188 @@ impl Scale { pub fn contains(&self, value: &Data) -> bool { match (&self.values, value) { (ScaleValues::Text(values), Data::Text(val)) => values.contains(val), - (ScaleValues::Number { start, end, .. }, Data::Number(num)) => { - start <= num && num <= end + (ScaleValues::Number { start, step, .. }, Data::Number(num)) => { + let end = start + (*step * (self.length - 1) as isize); + start <= num && num <= &end + } + (ScaleValues::Integer { start, step, .. }, Data::Integer(num)) => { + let end = start + (*step * (self.length - 1) as i32); + start <= num && num <= &end } - (ScaleValues::Integer { start, end, .. }, Data::Integer(num)) => { - start <= num && num <= end + (ScaleValues::Float { start, step, .. }, Data::Float(num)) => { + let end = start + (*step * (self.length - 1) as f32); + start <= num && num <= &end } - (ScaleValues::Float { start, end, .. }, Data::Float(num)) => start <= num && num <= end, _ => false, } } /// Assumes points is not empty - pub fn from_i32(points: impl Into>) -> Self { - let mut points: Vec = points.into(); - points.sort(); - - // Scale contains both negative and positive values - let temp = points.iter().fold((false, false), |acc, curr| { - if acc.0 && acc.1 { - acc - } else if *curr < 0 { - (true, acc.1) + fn from_i32(points: impl Iterator) -> Self { + let deduped = points.collect::>(); + + let mut min = None; + let mut max = None; + let mut has_neg = false; + let mut has_pos = false; + + for num in deduped.iter() { + let num = *num; + has_neg = num < 0; + has_pos = num >= 0; + + if let Some(prev) = min { + if num < prev { + min = Some(num); + } } else { - (acc.0, true) + min = Some(num); } - }); - let len = points.len(); - let min = points.first().unwrap(); // Guaranteed by iteration in Self::new - let max = points.get(len - 1).unwrap(); // Guaranteed by iteration in Self::new - let len = if temp.0 && temp.1 { - points.len() + 1 + if let Some(prev) = max { + if num > prev { + max = Some(num); + } + } else { + max = Some(num); + } + } + + let len = if has_pos && has_neg { + deduped.len() + 1 } else { - points.len() + deduped.len() }; + let min = min.unwrap(); + let max = max.unwrap(); let mut step = (max - min) / len as i32; if step * (len as i32) != max - min { step += 1 } + let length = if ((len - 1) as i32) * step + min < max { + len + 1 + } else { + len + }; + Self { kind: ScaleKind::Integer, + length, values: ScaleValues::Integer { - start: *min, - end: *max, + start: min, + end: max, step, }, } } /// Assumes points is not empty - pub fn from_isize(points: impl Into>) -> Self { - let mut points: Vec = points.into(); - points.sort(); - - let temp = points.iter().fold((false, false), |acc, curr| { - if acc.0 && acc.1 { - acc - } else if *curr < 0 { - (true, acc.1) + fn from_isize(points: impl Iterator) -> Self { + let deduped = points.collect::>(); + + let mut min = None; + let mut max = None; + let mut has_neg = false; + let mut has_pos = false; + + for num in deduped.iter() { + let num = *num; + has_neg = num < 0; + has_pos = num >= 0; + + if let Some(prev) = min { + if num < prev { + min = Some(num); + } } else { - (acc.0, true) + min = Some(num); } - }); - let len = points.len(); - let min = points.first().unwrap(); // Guaranteed by iteration in Self::new - let max = points.get(len - 1).unwrap(); // Guaranteed by iteration in Self::new - let len = if temp.0 && temp.1 { - points.len() + 1 + if let Some(prev) = max { + if num > prev { + max = Some(num); + } + } else { + max = Some(num); + } + } + + let len = if has_pos && has_neg { + deduped.len() + 1 } else { - points.len() + deduped.len() }; + let min = min.unwrap(); + let max = max.unwrap(); let mut step = (max - min) / len as isize; if step * (len as isize) != max - min { step += 1 } + let length = if ((len - 1) as isize) * step + min < max { + len + 1 + } else { + len + }; + Self { kind: ScaleKind::Number, + length, values: ScaleValues::Number { - start: *min, - end: *max, + start: min, + end: max, step, }, } } - pub fn from_f32(points: impl Into>) -> Self { - let points: Vec = points.into(); - - let mut min = *points.first().unwrap(); - let mut max = *points.first().unwrap(); + fn from_f32(points: impl Iterator) -> Self { + let mut min = None; + let mut max = None; + let mut has_neg = false; + let mut has_pos = false; + let mut seen = Vec::default(); - for point in points.iter() { - let point = *point; + for point in points { + let point = point; - if point < min { - min = point; - continue; + if !seen.iter().any(|pnt| *pnt == point) { + seen.push(point); } - if point > max { - max = point; - continue; + has_neg = point < 0.0; + has_pos = point >= 0.0; + + // I'm not quite certain how the < and > would work around NaN, + if let Some(prev) = min { + if point < prev { + min = Some(point); + } + } else { + min = Some(point); } - } - let temp = points.iter().fold((false, false), |acc, curr| { - if acc.0 && acc.1 { - acc - } else if *curr < 0.0 { - (true, acc.1) + if let Some(prev) = min { + if point > prev { + max = Some(point); + } } else { - (acc.0, true) + max = Some(point); } - }); + } - let len = if temp.0 && temp.1 { - points.len() + 1 + let min = min.unwrap(); + let max = max.unwrap(); + + let len = if has_pos && has_neg { + seen.len() + 1 } else { - points.len() + seen.len() }; let mut step = (max - min) / len as f32; @@ -348,8 +458,15 @@ impl Scale { step += 1.0 } + let length = if ((len - 1) as f32) * step + min < max { + len + 1 + } else { + len + }; + Self { kind: ScaleKind::Float, + length, values: ScaleValues::Float { start: min, end: max, @@ -367,22 +484,19 @@ impl Scale { impl From> for Scale { fn from(value: Vec) -> Self { - let values = value.into_iter().map(From::from); - Self::new(values, ScaleKind::Integer) + Self::new(value.into_iter(), ScaleKind::Integer) } } impl From> for Scale { fn from(value: Vec) -> Self { - let values = value.into_iter().map(From::from); - Self::new(values, ScaleKind::Number) + Self::new(value.into_iter(), ScaleKind::Number) } } impl From> for Scale { fn from(value: Vec) -> Self { - let values = value.into_iter().map(From::from); - Self::new(values, ScaleKind::Float) + Self::new(value.into_iter(), ScaleKind::Float) } } @@ -412,4 +526,166 @@ mod tests { assert_eq!(p4.x, "tired"); assert_eq!(p4.y, 0.50); } + + #[test] + fn test_scale_dedup() { + let pnts = vec![1, 2, 3, 4, 5]; + let scale = Scale::new(pnts, ScaleKind::Integer); + + assert_eq!(scale.length, 5); + assert_eq!( + scale.points(), + vec![ + Data::Integer(1), + Data::Integer(2), + Data::Integer(3), + Data::Integer(4), + Data::Integer(5) + ] + ); + + let pnts = vec![1, 3, 2, 2, 3, 4, 1, 5]; + let scale = Scale::new(pnts, ScaleKind::Integer); + + assert_eq!(scale.length, 5); + assert_eq!( + scale.points(), + vec![ + Data::Integer(1), + Data::Integer(2), + Data::Integer(3), + Data::Integer(4), + Data::Integer(5) + ] + ); + + let pnts: Vec = vec![1, 12, 12, 6, 4, 1, 25]; + let scale = Scale::new(pnts, ScaleKind::Number); + + assert_eq!(scale.length, 6); + assert!(scale.contains(&Data::Number(25))); + assert!(scale.contains(&Data::Number(26))); + assert!(!scale.contains(&Data::Number(30))); + assert!(!scale.contains(&Data::Integer(25))); + + let pnts: Vec = vec![1.0, 3.0, 2.0, 2.0, 3.0, 4.0, 1.0, 5.0]; + let scale = Scale::new(pnts, ScaleKind::Float); + + assert_eq!(scale.length, 6); + assert!(!scale.contains(&Data::Float(0.99))); + + let pnts: Vec = vec![1, 12, 12, 6, 4, 1, 25]; + let mut scale = Scale::new(pnts, ScaleKind::Text); + scale.sort(); + + assert_eq!(scale.length, 5); + assert_eq!( + scale.points(), + vec![ + Data::Text("1".into()), + Data::Text("12".into()), + Data::Text("25".into()), + Data::Text("4".into()), + Data::Text("6".into()), + ] + ); + + let pnts = vec![ + Data::Integer(44), + Data::Text("Test".into()), + Data::None, + Data::Integer(4), + ]; + let mut scale = Scale::new(pnts, ScaleKind::Integer); + scale.sort(); + + println!("{scale:?}"); + println!("{:?}", scale.points()); + + assert_eq!(scale.length, 4); + assert_eq!( + scale.points(), + vec![ + Data::Text("4".into()), + Data::Text("44".into()), + Data::Text("".into()), + Data::Text("Test".into()), + ] + ); + assert!(scale.contains(&Data::Text("44".into()))); + assert!(!scale.contains(&Data::Integer(44))); + assert!(!scale.contains(&Data::None)); + assert!(scale.contains(&Data::Text("Test".into()))); + assert!(scale.contains(&Data::Text("".into()))); + } + + #[test] + fn test_scale_pos_neg() { + let pnts = vec![-1, -8, -3]; + let scale = Scale::new(pnts, ScaleKind::Integer); + + assert_eq!(scale.length, 4); + assert_eq!( + scale.points(), + vec![ + Data::Integer(-8), + Data::Integer(-5), + Data::Integer(-2), + Data::Integer(1), + ] + ); + assert!(scale.contains(&Data::Integer(0))); + + let pnts = vec![-2, 0, 1, 2, 5]; + let scale = Scale::new(pnts, ScaleKind::Integer); + + assert_eq!(scale.length, 5); + assert_eq!( + scale.points(), + vec![ + Data::Integer(-2), + Data::Integer(0), + Data::Integer(2), + Data::Integer(4), + Data::Integer(6), + ] + ); + assert!(scale.contains(&Data::Integer(6))); + assert!(!scale.contains(&Data::Integer(-3))); + + let pnts = vec![-3, -10, -1, 2, -5]; + let scale = Scale::new(pnts, ScaleKind::Integer); + + assert_eq!(scale.length, 5); + assert_eq!( + scale.points(), + vec![ + Data::Integer(-10), + Data::Integer(-7), + Data::Integer(-4), + Data::Integer(-1), + Data::Integer(2), + ] + ); + } + + #[test] + fn test_scale_single() { + let pnt = vec![1]; + let scale = Scale::new(pnt, ScaleKind::Integer); + + assert_eq!(scale.length, 1); + assert_eq!(scale.points(), vec![Data::Integer(1)]); + assert!(scale.contains(&Data::Integer(1))); + assert!(!scale.contains(&Data::Integer(2))); + } + + #[test] + fn test_scale_none() { + let scale = Scale::new(Vec::::new(), ScaleKind::Integer); + + assert_eq!(scale.length, 1); + assert_eq!(scale.points(), vec![Data::Integer(0)]); + assert!(scale.contains(&Data::Integer(0))); + } } diff --git a/src/models/line.rs b/src/models/line.rs index 01f2fda..687f2db 100644 --- a/src/models/line.rs +++ b/src/models/line.rs @@ -173,7 +173,6 @@ mod line_tests { let y_scale = { let values = vec!["one", "two", "three", "four", "five"]; - let values = values.into_iter().map(ToOwned::to_owned).map(From::from); Scale::new(values, ScaleKind::Text) }; @@ -196,13 +195,11 @@ mod line_tests { let x_scale: Scale = { let rng = -5..11; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; let y_scale: Scale = { let rng = 2..10; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; diff --git a/src/models/stacked_bar.rs b/src/models/stacked_bar.rs index 8c28a09..0793d5b 100644 --- a/src/models/stacked_bar.rs +++ b/src/models/stacked_bar.rs @@ -349,7 +349,6 @@ mod stacked_barchart_tests { let x_scale = { let values = vec!["One", "Two", "Three", "Four", "Five"]; - let values = values.into_iter().map(ToOwned::to_owned).map(From::from); Scale::new(values, ScaleKind::Text) }; @@ -383,13 +382,11 @@ mod stacked_barchart_tests { let x_scale = { let rng = -5..11; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; let y_scale = { let rng = 2..10; - let rng = rng.into_iter().map(From::from); Scale::new(rng, ScaleKind::Integer) }; diff --git a/src/repr/sheet/tests.rs b/src/repr/sheet/tests.rs index 380bfcb..c66ce02 100644 --- a/src/repr/sheet/tests.rs +++ b/src/repr/sheet/tests.rs @@ -529,7 +529,7 @@ fn test_line_scales() { ) .expect("Building alter csv line graph failure"); - let expected_x_scale = { + let mut expected_x_scale = { let values = vec![1958, 1959, 1960]; let values = values.into_iter().map(|year| Data::Text(year.to_string())); @@ -537,6 +537,7 @@ fn test_line_scales() { }; line.x_scale.sort(); + expected_x_scale.sort(); assert_eq!(line.x_scale, expected_x_scale); let expected_y_scale = { From a643c460727241a9dec79b47b5aa41617215463d Mon Sep 17 00:00:00 2001 From: EmmanuelDodoo Date: Tue, 8 Oct 2024 00:49:45 +0000 Subject: [PATCH 3/3] lint: Applied clippy lints --- src/models/common.rs | 19 +++++-------------- src/repr/sheet.rs | 2 +- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/models/common.rs b/src/models/common.rs index faaef94..b5b441a 100644 --- a/src/models/common.rs +++ b/src/models/common.rs @@ -1,5 +1,5 @@ use crate::repr::{ColumnType, Data}; -use std::{collections::HashSet, fmt::Debug, isize}; +use std::{collections::HashSet, fmt::Debug}; #[derive(Debug, Clone, PartialEq)] pub struct Point { @@ -224,11 +224,7 @@ impl Scale { pub fn points(&self) -> Vec { match &self.values { - ScaleValues::Text(values) => values - .iter() - .cloned() - .map(|value| Data::Text(value)) - .collect(), + ScaleValues::Text(values) => values.iter().cloned().map(Data::Text).collect(), ScaleValues::Number { start, step, .. } => { let mut output = Vec::default(); let n = self.length as isize; @@ -416,8 +412,6 @@ impl Scale { let mut seen = Vec::default(); for point in points { - let point = point; - if !seen.iter().any(|pnt| *pnt == point) { seen.push(point); } @@ -484,19 +478,19 @@ impl Scale { impl From> for Scale { fn from(value: Vec) -> Self { - Self::new(value.into_iter(), ScaleKind::Integer) + Self::new(value, ScaleKind::Integer) } } impl From> for Scale { fn from(value: Vec) -> Self { - Self::new(value.into_iter(), ScaleKind::Number) + Self::new(value, ScaleKind::Number) } } impl From> for Scale { fn from(value: Vec) -> Self { - Self::new(value.into_iter(), ScaleKind::Float) + Self::new(value, ScaleKind::Float) } } @@ -599,9 +593,6 @@ mod tests { let mut scale = Scale::new(pnts, ScaleKind::Integer); scale.sort(); - println!("{scale:?}"); - println!("{:?}", scale.points()); - assert_eq!(scale.length, 4); assert_eq!( scale.points(), diff --git a/src/repr/sheet.rs b/src/repr/sheet.rs index 2cb49e3..7805cfb 100644 --- a/src/repr/sheet.rs +++ b/src/repr/sheet.rs @@ -1230,7 +1230,7 @@ impl Sheet { Scale::new(x_values, kind.into()) }; - let y_scale = Scale::new(y_values.into_iter(), y_kind); + let y_scale = Scale::new(y_values, y_kind); let acc_labels = acc_labels.into_iter().collect();