Rename "intTimePowerOfTen" to "singleDigitTimesPowerOfTen"; Silence w…

…arnings (#3727)
rilldata · Dec 19, 2023 · 3602e32 · 3602e32 · github-actions · Dec 19, 2023
1 parent cbbef38
commit 3602e32
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 23 deletions.
diff --git a/web-common/src/components/data-graphic/guides/Axis.svelte b/web-common/src/components/data-graphic/guides/Axis.svelte
@@ -3,7 +3,7 @@ This component will draw an axis on the specified side.
 -->
 <script lang="ts">
  import { NumberKind } from "@rilldata/web-common/lib/number-formatting/humanizer-types";
- import { IntTimesPowerOfTenFormatter } from "@rilldata/web-common/lib/number-formatting/strategies/IntTimesPowerOfTen";
+ import { SingleDigitTimesPowerOfTenFormatter } from "@rilldata/web-common/lib/number-formatting/strategies/SingleDigitTimesPowerOfTen";
  import { formatMsInterval } from "@rilldata/web-common/lib/number-formatting/strategies/intervals";
  import { getContext } from "svelte";
  import { contexts } from "../constants";
@@ -161,14 +161,15 @@ This component will draw an axis on the specified side.
  );
  } else {
  superlabel = false;
- // If this is a numeric axis, the d3 tick function used by
- // getTicks offers us some guarantees about the numbers returned.
- // In that case, we should be able to use the
- // IntTimesPowerOfTenFormatter, which is taylored to this situation.
- const formatter = new IntTimesPowerOfTenFormatter(ticks, {
- strategy: "intTimesPowerOfTen",
+ // Note that SingleDigitTimesPowerOfTenFormatter expects a single
+ // digit time a power of ten. The d3 axis function used upstream
+ // in this context _normally_ satisfies this requirement, but
+ // there are edge cases where it does not. In these edge cases,
+ // this formatter often does the right thing, but may not in some
+ // circumstances. See https:/rilldata/rill/issues/3631
+ const formatter = new SingleDigitTimesPowerOfTenFormatter(ticks, {
+ strategy: "singleDigitTimesPowerOfTen",
  numberKind,
- onInvalidInput: "consoleWarn",
  padWithInsignificantZeros: false,
  });
  formatterFunction = (x) =>

diff --git a/web-common/src/lib/number-formatting/humanizer-types.ts b/web-common/src/lib/number-formatting/humanizer-types.ts
@@ -129,7 +129,7 @@ export type FormatterOptionsNoneStrategy = {
  * of ten given as an input.
  */
 export type FormatterOptionsIntTimesPowerOfTenStrategy = {
- strategy: "intTimesPowerOfTen";
+ strategy: "singleDigitTimesPowerOfTen";
  onInvalidInput?: "doNothing" | "throw" | "consoleWarn";
 };
 

diff --git a/web-common/src/lib/number-formatting/humanizer.ts b/web-common/src/lib/number-formatting/humanizer.ts
@@ -1,5 +1,5 @@
 import { Formatter, FormatterFactory, NumberKind } from "./humanizer-types";
-import { IntTimesPowerOfTenFormatter } from "./strategies/IntTimesPowerOfTen";
+import { SingleDigitTimesPowerOfTenFormatter } from "./strategies/SingleDigitTimesPowerOfTen";
 import { NonFormatter } from "./strategies/none";
 import { IntervalFormatter } from "./strategies/intervals";
 import { PerRangeFormatter } from "./strategies/per-range";
@@ -52,8 +52,8 @@ export const humanizedFormatterFactory: FormatterFactory = (
  }
  break;
 
- case "intTimesPowerOfTen":
- formatter = new IntTimesPowerOfTenFormatter(sample, options);
+ case "singleDigitTimesPowerOfTen":
+ formatter = new SingleDigitTimesPowerOfTenFormatter(sample, options);
  break;
 
  default:

diff --git a/...ing/strategies/IntTimesPowerOfTen.spec.ts → ...tegies/SingleDigitTimesPowerOfTen.spec.ts b/...ing/strategies/IntTimesPowerOfTen.spec.ts → ...tegies/SingleDigitTimesPowerOfTen.spec.ts
@@ -1,12 +1,12 @@
 import { FormatterFactoryOptions, NumberKind } from "../humanizer-types";
 import {
- IntTimesPowerOfTenFormatter,
+ SingleDigitTimesPowerOfTenFormatter,
  closeToIntTimesPowerOfTen,
-} from "./IntTimesPowerOfTen";
+} from "./SingleDigitTimesPowerOfTen";
 import { describe, it, expect } from "vitest";
 
 const baseOptions: FormatterFactoryOptions = {
- strategy: "intTimesPowerOfTen",
+ strategy: "singleDigitTimesPowerOfTen",
  padWithInsignificantZeros: true,
  numberKind: NumberKind.ANY,
  onInvalidInput: "doNothing",
@@ -122,7 +122,7 @@ const testCases: [
 describe("default formatter, default options `.stringFormat()`", () => {
  testCases.forEach(([input, options, output]) => {
  it(`returns the correct split string in case: ${input}`, () => {
- const formatter = new IntTimesPowerOfTenFormatter([input], {
+ const formatter = new SingleDigitTimesPowerOfTenFormatter([input], {
  ...baseOptions,
  ...options,
  });
@@ -165,10 +165,10 @@ describe("default formatter, default options `.stringFormat()`", () => {
 // ];
 
 //FIXME re-enable this test when we have a better way to handle invalid inputs
-// describe("IntTimesPowerOfTenFormatter, returns empty NumberParts on invalid inputs", () => {
+// describe("SingleDigitTimesPowerOfTenFormatter, returns empty NumberParts on invalid inputs", () => {
 // errorCases.forEach(([input, options]) => {
 // it(`throws an error for input: ${input}`, () => {
-// const formatter = new IntTimesPowerOfTenFormatter([input], {
+// const formatter = new SingleDigitTimesPowerOfTenFormatter([input], {
 // ...baseOptions,
 // ...options,
 // ...{ onInvalidInput: "consoleWarn" },
@@ -187,10 +187,10 @@ const closeCases: [number, string][] = [
  [0.0030000000003, "3e-3"],
 ];
 
-describe("IntTimesPowerOfTenFormatter handles cases within an rounding error", () => {
+describe("SingleDigitTimesPowerOfTenFormatter handles cases within an rounding error", () => {
  closeCases.forEach(([input, output]) => {
  it(`returns the correct split string in case: ${input}, and does not throw an error`, () => {
- const formatter = new IntTimesPowerOfTenFormatter([input], {
+ const formatter = new SingleDigitTimesPowerOfTenFormatter([input], {
  ...baseOptions,
  ...{ onInvalidInput: "throw" },
  });

diff --git a/...rmatting/strategies/IntTimesPowerOfTen.ts → .../strategies/SingleDigitTimesPowerOfTen.ts b/...rmatting/strategies/IntTimesPowerOfTen.ts → .../strategies/SingleDigitTimesPowerOfTen.ts
@@ -43,7 +43,7 @@ export const closeToIntTimesPowerOfTen = (x: number) =>
  * or log a warning if a of a non integer multiple of a power
  * of ten given as an input.
  */
-export class IntTimesPowerOfTenFormatter implements Formatter {
+export class SingleDigitTimesPowerOfTenFormatter implements Formatter {
  options: FormatterOptionsCommon & FormatterOptionsIntTimesPowerOfTenStrategy;
  initialSample: number[];
 
@@ -75,7 +75,7 @@ export class IntTimesPowerOfTenFormatter implements Formatter {
  if (typeof x !== "number") {
  // FIXME add these warnings back in when the upstream code is robust enough
  // console.warn(
- // `Input to IntTimesPowerOfTenFormatter must be a number, got: ${x}. Returning empty NumberParts.`
+ // `Input to SingleDigitTimesPowerOfTenFormatter must be a number, got: ${x}. Returning empty NumberParts.`
  // );
  return { int: "", dot: "", frac: "", suffix: "" };
  }
@@ -95,7 +95,7 @@ export class IntTimesPowerOfTenFormatter implements Formatter {
  // valid inputs must already be close to a single digit
  // integer multiple of a power of ten
  if (!closeToIntTimesPowerOfTen(x)) {
- const msg = `received invalid input for IntTimesPowerOfTenFormatter: ${x}`;
+ const msg = `received invalid input for SingleDigitTimesPowerOfTenFormatter: ${x}`;
  if (onInvalidInput === "consoleWarn") {
  console.warn(msg);
  } else {