1882-Add min, max in addition to average for acting lists (#1969)

* Converting average.rs to math.rs

* Making some progress towards math

Examples and unit tests failing, also think I found a bug with passing in strings

* Fix typos

* Found issue with negative numbers

* Add some comments

* Split commands like in split and str_ but do not register?

* register commands in cli

* Address clippy warnings

* Fix bad examples

* Make the example failure message more helpful

* Replace unwraps

* Use compare_values to avoid coercion issues

* Remove unneeded code

crates/nu-cli/src/cli.rs

@@ -345,7 +345,10 @@ pub fn create_default_context(
             whole_stream_command(Headers),
             // Data processing
             whole_stream_command(Histogram),
+            whole_stream_command(Math),
             whole_stream_command(Average),
+            whole_stream_command(Minimum),
+            whole_stream_command(Maximum),
             whole_stream_command(Sum),
             // File format output
             whole_stream_command(To),

crates/nu-cli/src/commands.rs

@@ -8,7 +8,6 @@ pub(crate) mod alias;
 pub(crate) mod append;
 pub(crate) mod args;
 pub(crate) mod autoview;
-pub(crate) mod average;
 pub(crate) mod build_string;
 pub(crate) mod cal;
 pub(crate) mod calc;
@@ -68,6 +67,7 @@ pub(crate) mod lines;
 pub(crate) mod ls;
 #[allow(unused)]
 pub(crate) mod map_max_by;
+pub(crate) mod math;
 pub(crate) mod merge;
 pub(crate) mod mkdir;
 pub(crate) mod mv;
@@ -135,7 +135,6 @@ pub(crate) use command::{
 
 pub(crate) use alias::Alias;
 pub(crate) use append::Append;
-pub(crate) use average::Average;
 pub(crate) use build_string::BuildString;
 pub(crate) use cal::Cal;
 pub(crate) use calc::Calc;
@@ -151,6 +150,7 @@ pub(crate) use du::Du;
 pub(crate) use each::Each;
 pub(crate) use echo::Echo;
 pub(crate) use is_empty::IsEmpty;
+pub(crate) use math::Math;
 pub(crate) use update::Update;
 pub(crate) mod kill;
 pub(crate) use kill::Kill;
@@ -198,6 +198,7 @@ pub(crate) use lines::Lines;
 pub(crate) use ls::Ls;
 #[allow(unused_imports)]
 pub(crate) use map_max_by::MapMaxBy;
+pub(crate) use math::{Average, Maximum, Minimum};
 pub(crate) use merge::Merge;
 pub(crate) use mkdir::Mkdir;
 pub(crate) use mv::Move;

crates/nu-cli/src/commands/math/average.rs

@@ -1,28 +1,28 @@
+use crate::commands::math::utils::calculate;
 use crate::commands::WholeStreamCommand;
 use crate::prelude::*;
 use crate::utils::data_processing::{reducer_for, Reduce};
-use bigdecimal::FromPrimitive;
+use bigdecimal::{FromPrimitive, Zero};
 use nu_errors::ShellError;
-use nu_protocol::hir::{convert_number_to_u64, Number, Operator};
-use nu_protocol::{Dictionary, Primitive, ReturnSuccess, Signature, UntaggedValue, Value};
-use num_traits::identities::Zero;
+use nu_protocol::{
+    hir::{convert_number_to_u64, Number, Operator},
+    Primitive, Signature, UntaggedValue, Value,
+};
 
-use indexmap::map::IndexMap;
-
-pub struct Average;
+pub struct SubCommand;
 
 #[async_trait]
-impl WholeStreamCommand for Average {
+impl WholeStreamCommand for SubCommand {
     fn name(&self) -> &str {
-        "average"
+        "math average"
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("average")
+        Signature::build("math average")
     }
 
     fn usage(&self) -> &str {
-        "Average the values."
+        "Gets the average of a list of numbers"
     }
 
     async fn run(
@@ -30,75 +30,32 @@ impl WholeStreamCommand for Average {
         args: CommandArgs,
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
-        average(RunnableContext {
-            input: args.input,
-            registry: registry.clone(),
-            shell_manager: args.shell_manager,
-            host: args.host,
-            ctrl_c: args.ctrl_c,
-            current_errors: args.current_errors,
-            name: args.call_info.name_tag,
-            raw_input: args.raw_input,
-        })
+        calculate(
+            RunnableContext {
+                input: args.input,
+                registry: registry.clone(),
+                shell_manager: args.shell_manager,
+                host: args.host,
+                ctrl_c: args.ctrl_c,
+                current_errors: args.current_errors,
+                name: args.call_info.name_tag,
+                raw_input: args.raw_input,
+            },
+            average,
+        )
         .await
     }
 
     fn examples(&self) -> Vec<Example> {
         vec![Example {
-            description: "Average a list of numbers",
-            example: "echo [100 0 100 0] | average",
-            result: Some(vec![UntaggedValue::decimal(50).into()]),
+            description: "Get the average of a list of numbers",
+            example: "echo [-50 100.0 25] | math average",
+            result: Some(vec![UntaggedValue::decimal(25).into()]),
         }]
     }
 }
 
-async fn average(
-    RunnableContext {
-        mut input, name, ..
-    }: RunnableContext,
-) -> Result<OutputStream, ShellError> {
-    let values: Vec<Value> = input.drain_vec().await;
-
-    if values.iter().all(|v| v.is_primitive()) {
-        match avg(&values, name) {
-            Ok(result) => Ok(OutputStream::one(ReturnSuccess::value(result))),
-            Err(err) => Err(err),
-        }
-    } else {
-        let mut column_values = IndexMap::new();
-        for value in values {
-            if let UntaggedValue::Row(row_dict) = value.value {
-                for (key, value) in row_dict.entries.iter() {
-                    column_values
-                        .entry(key.clone())
-                        .and_modify(|v: &mut Vec<Value>| v.push(value.clone()))
-                        .or_insert(vec![value.clone()]);
-                }
-            }
-        }
-
-        let mut column_totals = IndexMap::new();
-        for (col_name, col_vals) in column_values {
-            match avg(&col_vals, &name) {
-                Ok(result) => {
-                    column_totals.insert(col_name, result);
-                }
-                Err(err) => return Err(err),
-            }
-        }
-
-        Ok(OutputStream::one(ReturnSuccess::value(
-            UntaggedValue::Row(Dictionary {
-                entries: column_totals,
-            })
-            .into_untagged_value(),
-        )))
-    }
-}
-
-fn avg(values: &[Value], name: impl Into<Tag>) -> Result<Value, ShellError> {
-    let name = name.into();
-
+pub fn average(values: &[Value], name: &Tag) -> Result<Value, ShellError> {
     let sum = reducer_for(Reduce::Sum);
 
     let number = BigDecimal::from_usize(values.len()).expect("expected a usize-sized bigdecimal");
@@ -156,12 +113,12 @@ fn avg(values: &[Value], name: impl Into<Tag>) -> Result<Value, ShellError> {
 
 #[cfg(test)]
 mod tests {
-    use super::Average;
+    use super::SubCommand;
 
     #[test]
     fn examples_work_as_expected() {
         use crate::examples::test as test_examples;
 
-        test_examples(Average {})
+        test_examples(SubCommand {})
     }
 }

crates/nu-cli/src/commands/math/command.rs

@@ -0,0 +1,135 @@
+use crate::commands::WholeStreamCommand;
+use crate::prelude::*;
+use nu_errors::ShellError;
+use nu_protocol::{ReturnSuccess, Signature, UntaggedValue};
+
+pub struct Command;
+
+#[async_trait]
+impl WholeStreamCommand for Command {
+    fn name(&self) -> &str {
+        "math"
+    }
+
+    fn signature(&self) -> Signature {
+        Signature::build("math")
+    }
+
+    fn usage(&self) -> &str {
+        r#"Use mathematical functions (average, min, max) to aggregate list of numbers or tables
+        math average
+        math min
+        math max
+        "#
+    }
+
+    async fn run(
+        &self,
+        _args: CommandArgs,
+        registry: &CommandRegistry,
+    ) -> Result<OutputStream, ShellError> {
+        Ok(OutputStream::one(Ok(ReturnSuccess::Value(
+            UntaggedValue::string(crate::commands::help::get_help(&Command, &registry.clone()))
+                .into_value(Tag::unknown()),
+        ))))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::commands::math::{
+        average::average, max::maximum, min::minimum, utils::MathFunction,
+    };
+    use nu_plugin::test_helpers::value::{decimal, int};
+    use nu_protocol::Value;
+
+    #[test]
+    fn examples_work_as_expected() {
+        use crate::examples::test as test_examples;
+
+        test_examples(Command {})
+    }
+
+    #[test]
+    fn test_math_functions() {
+        struct TestCase {
+            description: &'static str,
+            values: Vec<Value>,
+            expected_err: Option<ShellError>,
+            // Order is: avg, min, max
+            expected_res: Vec<Result<Value, ShellError>>,
+        }
+        let tt: Vec<TestCase> = vec![
+            TestCase {
+                description: "Empty data should throw an error",
+                values: Vec::new(),
+                expected_err: Some(ShellError::unexpected("Expected data")),
+                expected_res: Vec::new(),
+            },
+            TestCase {
+                description: "Single value",
+                values: vec![int(10)],
+                expected_err: None,
+                expected_res: vec![Ok(decimal(10)), Ok(int(10)), Ok(int(10))],
+            },
+            TestCase {
+                description: "Multiple Values",
+                values: vec![int(10), int(30), int(20)],
+                expected_err: None,
+                expected_res: vec![Ok(decimal(20)), Ok(int(10)), Ok(int(30))],
+            },
+            TestCase {
+                description: "Mixed Values",
+                values: vec![int(10), decimal(26.5), decimal(26.5)],
+                expected_err: None,
+                expected_res: vec![Ok(decimal(21)), Ok(int(10)), Ok(decimal(26.5))],
+            },
+            TestCase {
+                description: "Negative Values",
+                values: vec![int(10), int(-11), int(-14)],
+                expected_err: None,
+                expected_res: vec![Ok(decimal(-5)), Ok(int(-14)), Ok(int(10))],
+            },
+            TestCase {
+                description: "Mixed Negative Values",
+                values: vec![int(10), decimal(-11.5), decimal(-13.5)],
+                expected_err: None,
+                expected_res: vec![Ok(decimal(-5)), Ok(decimal(-13.5)), Ok(int(10))],
+            },
+            // TODO-Uncomment once Issue: https://github.com/nushell/nushell/issues/1883 is resolved
+            // TestCase {
+            //     description: "Invalid Mixed Values",
+            //     values: vec![int(10), decimal(26.5), decimal(26.5), string("math")],
+            //     expected_err: Some(ShellError::unimplemented("something")),
+            //     expected_res: vec![],
+            // },
+        ];
+        let test_tag = Tag::unknown();
+
+        for tc in tt.iter() {
+            let tc: &TestCase = tc; // Just for type annotations
+            let math_functions: Vec<MathFunction> = vec![average, minimum, maximum];
+            let results = math_functions
+                .iter()
+                .map(|mf| mf(&tc.values, &test_tag))
+                .collect_vec();
+
+            if tc.expected_err.is_some() {
+                assert!(
+                    results.iter().all(|r| r.is_err()),
+                    "Expected all functions to error for test-case: {}",
+                    tc.description,
+                );
+            } else {
+                for (i, res) in results.into_iter().enumerate() {
+                    assert_eq!(
+                        res, tc.expected_res[i],
+                        "math function {} failed on test-case {}",
+                        i, tc.description
+                    );
+                }
+            }
+        }
+    }
+}

crates/nu-cli/src/commands/math/max.rs

@@ -0,0 +1,69 @@
+use crate::commands::math::utils::calculate;
+use crate::commands::WholeStreamCommand;
+use crate::prelude::*;
+use crate::utils::data_processing::{reducer_for, Reduce};
+use nu_errors::ShellError;
+use nu_protocol::{Signature, UntaggedValue, Value};
+
+pub struct SubCommand;
+
+#[async_trait]
+impl WholeStreamCommand for SubCommand {
+    fn name(&self) -> &str {
+        "math max"
+    }
+
+    fn signature(&self) -> Signature {
+        Signature::build("math max")
+    }
+
+    fn usage(&self) -> &str {
+        "Get the maximum of a list of numbers or tables"
+    }
+
+    async fn run(
+        &self,
+        args: CommandArgs,
+        registry: &CommandRegistry,
+    ) -> Result<OutputStream, ShellError> {
+        calculate(
+            RunnableContext {
+                input: args.input,
+                registry: registry.clone(),
+                shell_manager: args.shell_manager,
+                host: args.host,
+                ctrl_c: args.ctrl_c,
+                current_errors: args.current_errors,
+                name: args.call_info.name_tag,
+                raw_input: args.raw_input,
+            },
+            maximum,
+        )
+        .await
+    }
+
+    fn examples(&self) -> Vec<Example> {
+        vec![Example {
+            description: "Find the maximum of list of numbers",
+            example: "echo [-50 100 25] | math max",
+            result: Some(vec![UntaggedValue::int(100).into()]),
+        }]
+    }
+}
+
+pub fn maximum(values: &[Value], _name: &Tag) -> Result<Value, ShellError> {
+    let max_func = reducer_for(Reduce::Maximum);
+    max_func(Value::nothing(), values.to_vec())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::SubCommand;
+
+    #[test]
+    fn examples_work_as_expected() {
+        use crate::examples::test as test_examples;
+
+        test_examples(SubCommand {})
+    }
+}

crates/nu-cli/src/commands/math/min.rs

@@ -0,0 +1,69 @@
+use crate::commands::math::utils::calculate;
+use crate::commands::WholeStreamCommand;
+use crate::prelude::*;
+use crate::utils::data_processing::{reducer_for, Reduce};
+use nu_errors::ShellError;
+use nu_protocol::{Signature, UntaggedValue, Value};
+
+pub struct SubCommand;
+
+#[async_trait]
+impl WholeStreamCommand for SubCommand {
+    fn name(&self) -> &str {
+        "math min"
+    }
+
+    fn signature(&self) -> Signature {
+        Signature::build("math min")
+    }
+
+    fn usage(&self) -> &str {
+        "Finds the minimum within a list of numbers or tables"
+    }
+
+    async fn run(
+        &self,
+        args: CommandArgs,
+        registry: &CommandRegistry,
+    ) -> Result<OutputStream, ShellError> {
+        calculate(
+            RunnableContext {
+                input: args.input,
+                registry: registry.clone(),
+                shell_manager: args.shell_manager,
+                host: args.host,
+                ctrl_c: args.ctrl_c,
+                current_errors: args.current_errors,
+                name: args.call_info.name_tag,
+                raw_input: args.raw_input,
+            },
+            minimum,
+        )
+        .await
+    }
+
+    fn examples(&self) -> Vec<Example> {
+        vec![Example {
+            description: "Get the minimum of a list of numbers",
+            example: "echo [-50 100 25] | math min",
+            result: Some(vec![UntaggedValue::int(-50).into()]),
+        }]
+    }
+}
+
+pub fn minimum(values: &[Value], _name: &Tag) -> Result<Value, ShellError> {
+    let min_func = reducer_for(Reduce::Minimum);
+    min_func(Value::nothing(), values.to_vec())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::SubCommand;
+
+    #[test]
+    fn examples_work_as_expected() {
+        use crate::examples::test as test_examples;
+
+        test_examples(SubCommand {})
+    }
+}

crates/nu-cli/src/commands/math/mod.rs

@@ -0,0 +1,10 @@
+pub mod average;
+pub mod command;
+pub mod max;
+pub mod min;
+pub mod utils;
+
+pub use average::SubCommand as Average;
+pub use command::Command as Math;
+pub use max::SubCommand as Maximum;
+pub use min::SubCommand as Minimum;

crates/nu-cli/src/commands/math/utils.rs

@@ -0,0 +1,52 @@
+use crate::prelude::*;
+use nu_errors::ShellError;
+use nu_protocol::{Dictionary, ReturnSuccess, UntaggedValue, Value};
+
+use indexmap::map::IndexMap;
+
+pub type MathFunction = fn(values: &[Value], tag: &Tag) -> Result<Value, ShellError>;
+
+pub async fn calculate(
+    RunnableContext {
+        mut input, name, ..
+    }: RunnableContext,
+    mf: MathFunction,
+) -> Result<OutputStream, ShellError> {
+    let values: Vec<Value> = input.drain_vec().await;
+
+    if values.iter().all(|v| v.is_primitive()) {
+        match mf(&values, &name) {
+            Ok(result) => Ok(OutputStream::one(ReturnSuccess::value(result))),
+            Err(err) => Err(err),
+        }
+    } else {
+        let mut column_values = IndexMap::new();
+        for value in values {
+            if let UntaggedValue::Row(row_dict) = value.value {
+                for (key, value) in row_dict.entries.iter() {
+                    column_values
+                        .entry(key.clone())
+                        .and_modify(|v: &mut Vec<Value>| v.push(value.clone()))
+                        .or_insert(vec![value.clone()]);
+                }
+            }
+        }
+
+        let mut column_totals = IndexMap::new();
+        for (col_name, col_vals) in column_values {
+            match mf(&col_vals, &name) {
+                Ok(result) => {
+                    column_totals.insert(col_name, result);
+                }
+                Err(err) => return Err(err),
+            }
+        }
+
+        Ok(OutputStream::one(ReturnSuccess::value(
+            UntaggedValue::Row(Dictionary {
+                entries: column_totals,
+            })
+            .into_untagged_value(),
+        )))
+    }
+}

crates/nu-cli/src/data/value.rs

@@ -126,6 +126,7 @@ pub fn compute_values(
     }
 }
 
+/// If left is {{ Operator }} right
 pub fn compare_values(
     operator: Operator,
     left: &UntaggedValue,

crates/nu-cli/src/examples.rs

@@ -29,8 +29,10 @@ pub fn test(cmd: impl WholeStreamCommand + 'static) {
                     .iter()
                     .zip(result.iter())
                     .all(|(e, a)| values_equal(e, a)),
-                "example produced unexpected result: {}",
+                "example command produced unexpected result.\ncommand: {}\nexpected: {:?}\nactual:{:?}",
                 example.example,
+                expected,
+                result,
             );
         }
     }

crates/nu-cli/src/utils/data_processing.rs

@@ -8,6 +8,9 @@ use nu_source::{SpannedItem, Tag, Tagged, TaggedItem};
 use nu_value_ext::{get_data_by_key, ValueExt};
 use num_traits::Zero;
 
+// Re-usable error messages
+const ERR_EMPTY_DATA: &str = "Cannot perform aggregate math operation on empty data";
+
 pub fn columns_sorted(
     _group_by_name: Option<String>,
     value: &Value,
@@ -198,6 +201,9 @@ pub fn evaluate(
 }
 
 pub fn sum(data: Vec<Value>) -> Result<Value, ShellError> {
+    if data.is_empty() {
+        return Err(ShellError::unexpected(ERR_EMPTY_DATA));
+    }
     let mut acc = Value::zero();
     for value in data {
         match value.value {
@@ -214,6 +220,56 @@ pub fn sum(data: Vec<Value>) -> Result<Value, ShellError> {
     Ok(acc)
 }
 
+pub fn max(data: Vec<Value>) -> Result<Value, ShellError> {
+    let mut biggest = data
+        .first()
+        .ok_or_else(|| ShellError::unexpected(ERR_EMPTY_DATA))?
+        .value
+        .clone();
+
+    for value in data.iter() {
+        if let Ok(greater_than) = compare_values(Operator::GreaterThan, &value.value, &biggest) {
+            if greater_than {
+                biggest = value.value.clone();
+            }
+        } else {
+            return Err(ShellError::unexpected(format!(
+                "Could not compare\nleft: {:?}\nright: {:?}",
+                biggest, value.value
+            )));
+        }
+    }
+    Ok(Value {
+        value: biggest,
+        tag: Tag::unknown(),
+    })
+}
+
+pub fn min(data: Vec<Value>) -> Result<Value, ShellError> {
+    let mut smallest = data
+        .first()
+        .ok_or_else(|| ShellError::unexpected(ERR_EMPTY_DATA))?
+        .value
+        .clone();
+
+    for value in data.iter() {
+        if let Ok(greater_than) = compare_values(Operator::LessThan, &value.value, &smallest) {
+            if greater_than {
+                smallest = value.value.clone();
+            }
+        } else {
+            return Err(ShellError::unexpected(format!(
+                "Could not compare\nleft: {:?}\nright: {:?}",
+                smallest, value.value
+            )));
+        }
+    }
+    Ok(Value {
+        value: smallest,
+        tag: Tag::unknown(),
+    })
+}
+
 fn formula(
     acc_begin: Value,
     calculator: Box<dyn Fn(Vec<Value>) -> Result<Value, ShellError> + Send + Sync + 'static>,
@@ -233,11 +289,15 @@ pub fn reducer_for(
 ) -> Box<dyn Fn(Value, Vec<Value>) -> Result<Value, ShellError> + Send + Sync + 'static> {
     match command {
         Reduce::Sum | Reduce::Default => Box::new(formula(Value::zero(), Box::new(sum))),
+        Reduce::Minimum => Box::new(|_, values| min(values)),
+        Reduce::Maximum => Box::new(|_, values| max(values)),
     }
 }
 
 pub enum Reduce {
     Sum,
+    Minimum,
+    Maximum,
     Default,
 }
 
@@ -250,6 +310,8 @@ pub fn reduce(
 
     let reduce_with = match reducer {
         Some(cmd) if cmd == "sum" => reducer_for(Reduce::Sum),
+        Some(cmd) if cmd == "min" => reducer_for(Reduce::Minimum),
+        Some(cmd) if cmd == "max" => reducer_for(Reduce::Maximum),
         Some(_) | None => reducer_for(Reduce::Default),
     };
 

crates/nu-cli/tests/commands/average.rs

@@ -7,11 +7,11 @@ fn can_average_numbers() {
         r#"
              open sgml_description.json
              | get glossary.GlossDiv.GlossList.GlossEntry.Sections
-             | average
+             | math average
              | echo $it
          "#
     ));
-
+    println!("{:?}", actual.err);
     assert_eq!(actual.out, "101.5")
 }
 
@@ -19,7 +19,7 @@ fn can_average_numbers() {
 fn can_average_bytes() {
     let actual = nu!(
         cwd: "tests/fixtures/formats",
-        "ls | sort-by name | skip 1 | first 2 | get size | average | format \"{$it}\" | echo $it"
+        "ls | sort-by name | skip 1 | first 2 | get size | math average | format \"{$it}\" | echo $it"
     );
 
     assert_eq!(actual.out, "1.6 KB");