Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Round to 9 decimal places for formulas w/o a user-set decimal places #893

Merged
merged 8 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions quadratic-client/src/grid/controller/Grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,12 @@ export class Grid {
//#region Summarize
//-----------------

summarizeSelection() {
return this.gridController.summarizeSelection(sheets.sheet.id, rectangleToRect(sheets.sheet.cursor.getRectangle()));
summarizeSelection(decimal_places: number = 9) {
return this.gridController.summarizeSelection(
sheets.sheet.id,
rectangleToRect(sheets.sheet.cursor.getRectangle()),
BigInt(decimal_places)
);
}

//#endregion
Expand Down
1 change: 1 addition & 0 deletions quadratic-client/src/gridGL/helpers/selectCells.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { sheets } from '../../grid/controller/Sheets';
export function selectAllCells(): void {
const sheet = sheets.sheet;
const bounds = grid.getGridBounds(sheet.id, true);

if (bounds) {
sheet.cursor.changePosition({
multiCursor: {
Expand Down
5 changes: 3 additions & 2 deletions quadratic-client/src/ui/menus/BottomBar/SelectionSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import BottomBarItem from './BottomBarItem';

export const SelectionSummary = () => {
const cursor = sheets.sheet.cursor;
const decimal_places = 9;

const isBigEnoughForActiveSelectionStats = useMediaQuery('(min-width:1000px)');
const [count, setCount] = useState<string | undefined>('');
Expand All @@ -16,12 +17,12 @@ export const SelectionSummary = () => {
const [copied, setCopied] = useState(false);

const runCalculationOnActiveSelection = () => {
let result = grid.summarizeSelection();
let result = grid.summarizeSelection(decimal_places);

if (result) {
setCount(result.count.toString());
setSum(result.sum !== undefined ? result.sum.toString() : undefined);
setAvg(result.average !== undefined ? result.average.toFixed(9).toString() : undefined);
setAvg(result.average !== undefined ? result.average.toString() : undefined);
} else {
setCount(undefined);
setSum(undefined);
Expand Down
61 changes: 29 additions & 32 deletions quadratic-core/src/grid/sheet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,10 +460,13 @@ impl Sheet {
match value {
CellValue::Number(n) => {
let (_, exponent) = n.as_bigint_and_exponent();
let max_decimals = 9;
let decimals = exponent.min(max_decimals) as i16;

if is_percentage {
Some(exponent as i16 - 2)
Some(decimals - 2)
} else {
Some(exponent as i16)
Some(decimals)
}
}
_ => None,
Expand Down Expand Up @@ -570,22 +573,35 @@ mod test {
(grid_controller, sheet_id, selected)
}

// assert decimal places after a set_cell_value
fn assert_decimal_places_for_number(
sheet: &mut Sheet,
x: i64,
y: i64,
value: &str,
is_percentage: bool,
expected: Option<i16>,
) {
let pos = Pos { x, y };
sheet.set_cell_value(pos, CellValue::Number(BigDecimal::from_str(value).unwrap()));
assert_eq!(sheet.decimal_places(pos, is_percentage), expected);
}

#[test]
fn test_current_decimal_places_value() {
let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from(""));

// get decimal places after a set_cell_value
sheet.set_cell_value(
Pos { x: 1, y: 2 },
CellValue::Number(BigDecimal::from_str("12.23").unwrap()),
);
assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, false), Some(2));
// validate simple decimal places
assert_decimal_places_for_number(&mut sheet, 1, 2, "12.23", false, Some(2));

sheet.set_cell_value(
Pos { x: 2, y: 2 },
CellValue::Number(BigDecimal::from_str("0.23").unwrap()),
);
assert_eq!(sheet.decimal_places(Pos { x: 2, y: 2 }, true), Some(0));
// validate percentage
assert_decimal_places_for_number(&mut sheet, 2, 2, "0.23", true, Some(0));

// validate rounding
assert_decimal_places_for_number(&mut sheet, 3, 2, "9.1234567891", false, Some(9));

// validate percentage rounding
assert_decimal_places_for_number(&mut sheet, 3, 2, "9.1234567891", true, Some(7));
}

#[test]
Expand All @@ -610,25 +626,6 @@ mod test {
assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, false), None);
}

#[test]
fn test_current_decimal_places_percent() {
let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from(""));

sheet.set_cell_value(
crate::Pos { x: 1, y: 2 },
CellValue::Number(BigDecimal::from_str("0.24").unwrap()),
);

assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, true), Some(0));

sheet.set_cell_value(
crate::Pos { x: 1, y: 2 },
CellValue::Number(BigDecimal::from_str("0.245").unwrap()),
);

assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, true), Some(1));
}

#[test]
fn test_cell_numeric_format_kind() {
let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from(""));
Expand Down
14 changes: 14 additions & 0 deletions quadratic-core/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ pub fn date_string() -> String {
now.format("%Y-%m-%d %H:%M:%S").to_string()
}

pub fn round(number: f64, precision: i64) -> f64 {
let y = 10i32.pow(precision as u32) as f64;
(number * y).round() / y
}

#[cfg(test)]
pub(crate) fn assert_f64_approx_eq(expected: f64, actual: &str) {
const EPSILON: f64 = 0.0001;
Expand Down Expand Up @@ -433,4 +438,13 @@ mod tests {
fn test_date_string() {
assert_eq!(date_string().len(), 19);
}

#[test]
fn test_round() {
assert_eq!(round(1.23456789, 0), 1.0);
assert_eq!(round(1.23456789, 1), 1.2);
assert_eq!(round(1.23456789, 2), 1.23);
assert_eq!(round(1.23456789, 3), 1.235);
assert_eq!(round(1.23456789, 4), 1.2346);
}
}
55 changes: 42 additions & 13 deletions quadratic-core/src/wasm_bindings/controller/summarize.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::util::round;
use crate::Pos;
use crate::{grid::SheetId, wasm_bindings::GridController, CellValue, Rect};
use bigdecimal::{BigDecimal, ToPrimitive, Zero};
use std::str::FromStr;
use wasm_bindgen::prelude::wasm_bindgen;

const MAXIMUM_SUMMARIZE_SELECTION_SIZE: u32 = 50000;
const MAX_SUMMARIZE_SELECTION_SIZE: u32 = 50000;

#[wasm_bindgen]
pub struct SummarizeSelectionResult {
Expand All @@ -20,9 +21,10 @@ impl GridController {
&mut self,
sheet_id: String,
rect: &Rect,
max_decimals: i64,
) -> Option<SummarizeSelectionResult> {
// don't allow too large of a selection
if (rect.width() * rect.height()) > MAXIMUM_SUMMARIZE_SELECTION_SIZE {
if rect.len() > MAX_SUMMARIZE_SELECTION_SIZE {
return None;
}

Expand Down Expand Up @@ -60,8 +62,8 @@ impl GridController {

Some(SummarizeSelectionResult {
count,
sum: sum.to_f64(),
average: average.to_f64(),
sum: sum.to_f64().map(|num| round(num, max_decimals)),
average: average.to_f64().map(|num| round(num, max_decimals)),
})
}
}
Expand All @@ -72,35 +74,62 @@ mod tests {
use crate::wasm_bindings::GridController;
use crate::{Pos, Rect};

// TODO(ddimaria): move to a shared util
fn set_value(gc: &mut GridController, x: i64, y: i64, value: &str) {
let sheet_id = gc.sheet_ids()[0];
gc.set_cell_value(sheet_id, Pos { x, y }, String::from(value), None);
}

#[test]
fn test_summarize() {
let mut gc = GridController::default();
let sheet_id = gc.sheet_ids()[0];

gc.set_cell_value(sheet_id, Pos { x: 1, y: 1 }, String::from("12.12"), None);
gc.set_cell_value(sheet_id, Pos { x: 1, y: 2 }, String::from("12313"), None);
gc.set_cell_value(sheet_id, Pos { x: 1, y: 3 }, String::from("0"), None);
set_value(&mut gc, 1, 1, "12.12");
set_value(&mut gc, 1, 2, "12313");
set_value(&mut gc, 1, 3, "0");

// span of 10 cells, 3 have numeric values
let rect = Rect::new_span(Pos { x: 1, y: 1 }, Pos { x: 1, y: 10 });
let result = gc
.js_summarize_selection(sheet_id.to_string(), &rect)
.js_summarize_selection(sheet_id.to_string(), &rect, 9)
.unwrap();

assert_eq!(result.count, 3);
assert_eq!(result.sum, Some(12325.12));
assert_eq!(result.average, Some(4108.373333333333));
assert_eq!(result.average, Some(4108.373333333));

// returns zeros for an empty selection
let rect = Rect::new_span(Pos { x: 100, y: 100 }, Pos { x: 1000, y: 105 });
let result = gc
.js_summarize_selection(sheet_id.to_string(), &rect)
.js_summarize_selection(sheet_id.to_string(), &rect, 9)
.unwrap();

assert_eq!(result.count, 0);
assert_eq!(result.sum, Some(0.0));
assert_eq!(result.average, Some(0.0));

// returns none if selection is too large (MAX_SUMMARIZE_SELECTION_SIZE)
let rect = Rect::new_span(Pos { x: 100, y: 100 }, Pos { x: 10000, y: 10000 });
let result = gc.js_summarize_selection(sheet_id.to_string(), &rect);
let result = gc.js_summarize_selection(sheet_id.to_string(), &rect, 9);
assert!(result.is_none());

// rounding
set_value(&mut gc, 1, 1, "9.1234567891");
let rect = Rect::new_span(Pos { x: 1, y: 1 }, Pos { x: 1, y: 10 });
let result = gc
.js_summarize_selection(sheet_id.to_string(), &rect, 9)
.unwrap();
assert_eq!(result.count, 3);
assert_eq!(result.sum, Some(12322.123456789));
assert_eq!(result.average, Some(4107.374485596));

// trailing zeros
set_value(&mut gc, -1, -1, "0.00100000000000");
let rect = Rect::new_span(Pos { x: -1, y: -1 }, Pos { x: -1, y: -10 });
let result = gc
.js_summarize_selection(sheet_id.to_string(), &rect, 9)
.unwrap();
assert_eq!(result.count, 1);
assert_eq!(result.sum, Some(0.001));
assert_eq!(result.average, Some(0.001));
}
}