From d00bf9c42a69c32cc5fa0cccba2370548b120a3b Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 18 Jan 2024 08:38:21 -0500 Subject: [PATCH 1/4] New rounding rule --- src/diagram/models/variable.ts | 4 +++- src/diagram/utils/math-utils.test.ts | 21 +++++++++++++++++++++ src/diagram/utils/math-utils.ts | 16 ++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/diagram/utils/math-utils.test.ts create mode 100644 src/diagram/utils/math-utils.ts diff --git a/src/diagram/models/variable.ts b/src/diagram/models/variable.ts index f2c500d..9e4132f 100644 --- a/src/diagram/models/variable.ts +++ b/src/diagram/models/variable.ts @@ -6,6 +6,7 @@ import { getMathUnit, getUsedInputs, replaceInputNames } from "../utils/mathjs-u import { createMath } from "../custom-mathjs"; import { basicErrorMessage, ErrorMessage, getErrorMessage } from "../utils/error"; import { Colors, legacyColors } from "../utils/theme-utils"; +import { roundForDisplay } from "../utils/math-utils"; export enum Operation { Divide = "รท", @@ -339,9 +340,10 @@ export const Variable = types.model("Variable", { if (value === undefined) { return "NaN"; } + const rounded = roundForDisplay(value, 3); // The first argument is the locale, using undefined means it should pick up the default // browser locale - return new Intl.NumberFormat(undefined, { maximumFractionDigits: 4 }).format(value); + return new Intl.NumberFormat(undefined, { maximumFractionDigits: 20 }).format(rounded); }, get computedValueError() { return self.computedValueIncludingMessageAndError.error; diff --git a/src/diagram/utils/math-utils.test.ts b/src/diagram/utils/math-utils.test.ts new file mode 100644 index 0000000..88cf643 --- /dev/null +++ b/src/diagram/utils/math-utils.test.ts @@ -0,0 +1,21 @@ +import { roundForDisplay } from "./math-utils"; + +describe("roundForDisplay", () => { + it("should respect the number of decimal places requested", () => { + expect(roundForDisplay(Math.PI, 4)).toBe(3.142); + expect(roundForDisplay(Math.PI, 3)).toBe(3.14); + expect(roundForDisplay(Math.PI, 2)).toBe(3.1); + expect(roundForDisplay(Math.PI, 1)).toBe(3); + }); + it("should handle very small numbers", () => { + expect(roundForDisplay(.00000039234, 3)).toBe(0.000000392); + expect(roundForDisplay(.00000039234, 2)).toBe(0.00000039); + expect(roundForDisplay(.00000039234, 1)).toBe(0.0000004); + }); + it("should never remove units", () => { + expect(roundForDisplay(123456.1, 3)).toBe(123456); + expect(roundForDisplay(123456.1, 2)).toBe(123456); + expect(roundForDisplay(123456.1, 1)).toBe(123456); + expect(roundForDisplay(17.6, 1)).toBe(18); + }); +}); diff --git a/src/diagram/utils/math-utils.ts b/src/diagram/utils/math-utils.ts new file mode 100644 index 0000000..292bb17 --- /dev/null +++ b/src/diagram/utils/math-utils.ts @@ -0,0 +1,16 @@ +/** + * Rounds a number in an intuitive way. + * The digits parameter is used as a number of significant digits to maintain for small numbers. + * However, rounding never carries beyond the integer part of a number, even if it contains more than + * that number of significant digits. + * @param x number to round + * @param digits number of digits of precision to maintain in decimals + * @returns a rounded number + */ +export function roundForDisplay(x: number, digits: number): number { + const intRange = Math.pow(10, digits); + if (x<-intRange || x>intRange) { + return Math.round(x); + } + return +x.toPrecision(digits); +} From b236b2f1adf741d019853b6c9de49401746e0b25 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 18 Jan 2024 08:54:42 -0500 Subject: [PATCH 2/4] Version number update --- diagram-view/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diagram-view/package.json b/diagram-view/package.json index 095bac0..af3db2c 100644 --- a/diagram-view/package.json +++ b/diagram-view/package.json @@ -1,6 +1,6 @@ { "name": "@concord-consortium/diagram-view", - "version": "0.0.53", + "version": "0.0.54", "description": "Concord Consortium Diagram View", "main": "dist/index.js", "module": "dist/index.esm.js", From 92297e5a7a5319eb72b82dbe071ae55e08ca0034 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 18 Jan 2024 09:03:28 -0500 Subject: [PATCH 3/4] Package-lock update --- diagram-view/package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diagram-view/package-lock.json b/diagram-view/package-lock.json index 51a724e..3d3b80f 100644 --- a/diagram-view/package-lock.json +++ b/diagram-view/package-lock.json @@ -1,12 +1,12 @@ { "name": "@concord-consortium/diagram-view", - "version": "0.0.52", + "version": "0.0.54", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@concord-consortium/diagram-view", - "version": "0.0.52", + "version": "0.0.54", "license": "MIT", "dependencies": { "iframe-phone": "^1.3.1", From d030f0c99a8b8b6832606c5b016bbc125aa5c815 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 18 Jan 2024 09:03:46 -0500 Subject: [PATCH 4/4] Fix tests for new rounding --- src/diagram/components/ui/variable-chip.test.tsx | 12 ++++++------ src/diagram/models/variable.test.ts | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/diagram/components/ui/variable-chip.test.tsx b/src/diagram/components/ui/variable-chip.test.tsx index 1ef1f56..f4575fc 100644 --- a/src/diagram/components/ui/variable-chip.test.tsx +++ b/src/diagram/components/ui/variable-chip.test.tsx @@ -15,22 +15,22 @@ describe("VariableChip", () => { expect(container).toHaveTextContent(/^some name$/); applySnapshot(variable, {id: variable.id, value: 1.234}); - expect(container).toHaveTextContent(/^1.234$/); + expect(container).toHaveTextContent(/^1.23$/); applySnapshot(variable, {id: variable.id, unit: "m"}); expect(container).toHaveTextContent(/^\(m\)$/); applySnapshot(variable, {id: variable.id, name: "some name", value: 1.234}); - expect(container).toHaveTextContent(/^some name=1.234$/); + expect(container).toHaveTextContent(/^some name=1.23$/); applySnapshot(variable, {id: variable.id, name: "some name", unit: "m"}); expect(container).toHaveTextContent(/^some name\(m\)$/); applySnapshot(variable, {id: variable.id, value: 1.234, unit: "m" }); - expect(container).toHaveTextContent(/^1.234m$/); + expect(container).toHaveTextContent(/^1.23m$/); applySnapshot(variable, {id: variable.id, name: "some name", value: 1.234, unit: "m" }); - expect(container).toHaveTextContent(/^some name=1.234m$/); + expect(container).toHaveTextContent(/^some name=1.23m$/); }); it("properly renders name only variable chips", () => { const variable = Variable.create(); @@ -41,7 +41,7 @@ describe("VariableChip", () => { expect(container).toHaveTextContent(/^some name$/); applySnapshot(variable, {id: variable.id, value: 1.234}); - expect(container).toHaveTextContent(/^1.234$/); + expect(container).toHaveTextContent(/^1.23$/); applySnapshot(variable, {id: variable.id, unit: "m"}); expect(container).toHaveTextContent(/^\(m\)$/); @@ -53,7 +53,7 @@ describe("VariableChip", () => { expect(container).toHaveTextContent(/^some name$/); applySnapshot(variable, {id: variable.id, value: 1.234, unit: "m" }); - expect(container).toHaveTextContent(/^1.234m$/); + expect(container).toHaveTextContent(/^1.23m$/); applySnapshot(variable, {id: variable.id, name: "some name", value: 1.234, unit: "m" }); expect(container).toHaveTextContent(/^some name$/); diff --git a/src/diagram/models/variable.test.ts b/src/diagram/models/variable.test.ts index eced4fe..fb6e463 100644 --- a/src/diagram/models/variable.test.ts +++ b/src/diagram/models/variable.test.ts @@ -30,7 +30,7 @@ describe("Variable", () => { expect(variable.computedValueIncludingMessageAndError).toEqual({value: 123.5}); expect(variable.computedUnitIncludingMessageAndError).toEqual({}); expect(variable.computedValue).toBe(123.5); - expect(variable.computedValueWithSignificantDigits).toBe("123.5"); + expect(variable.computedValueWithSignificantDigits).toBe("124"); expect(variable.computedValueError).toBeUndefined(); expect(variable.computedValueMessage).toBeUndefined(); expect(variable.computedUnit).toBeUndefined(); @@ -177,7 +177,7 @@ describe("Variable", () => { expect(variable.computedValueIncludingMessageAndError).toEqual({value: 999.9}); expect(variable.computedUnitIncludingMessageAndError).toEqual({unit: "mm"}); expect(variable.computedValue).toBe(999.9); - expect(variable.computedValueWithSignificantDigits).toBe("999.9"); + expect(variable.computedValueWithSignificantDigits).toBe("1,000"); expect(variable.computedValueError).toBeUndefined(); expect(variable.computedUnit).toBe("mm"); expect(variable.computedUnitError).toBeUndefined();