Scale.log fixes

packages/components/src/lib/d3/patchedScales.ts

@@ -55,11 +55,13 @@ function tickFormatWithCustom(
   return d3.tickFormat(start, stop, count, specifier);
 }
 
+type ScaleLinear = d3.ScaleLinear<number, number, never>;
+type ScaleLogarithmic = d3.ScaleLogarithmic<number, number, never>;
+type ScaleSymLog = d3.ScaleSymLog<number, number, never>;
+type ScalePower = d3.ScalePower<number, number, never>;
+
 function patchLinearishTickFormat<
-  T extends
-    | d3.ScaleLinear<number, number, never>
-    | d3.ScaleSymLog<number, number, never>
-    | d3.ScalePower<number, number, never>,
+  T extends ScaleLinear | ScaleSymLog | ScalePower,
 >(scale: T): T {
   // copy-pasted from https://github.com/d3/d3-scale/blob/83555bd759c7314420bd4240642beda5e258db9e/src/linear.js#L14
   scale.tickFormat = (count, specifier) => {
@@ -70,23 +72,11 @@ function patchLinearishTickFormat<
   return scale;
 }
 
-// Original d3.scale* should never be used; they won't support our custom tick formats.
-
-export function scaleLinear() {
-  return patchLinearishTickFormat(d3.scaleLinear());
-}
-
-export function scaleSymlog() {
-  return patchLinearishTickFormat(d3.scaleSymlog());
+function patchSymlogTickFormat(scale: ScaleSymLog): ScaleSymLog {
+  return patchLinearishTickFormat(scale);
 }
 
-export function scalePow() {
-  return patchLinearishTickFormat(d3.scalePow());
-}
-
-export function scaleLog() {
-  // log scale tickFormat is special
-  const scale = d3.scaleLog();
+function patchLogarithmicTickFormat(scale: ScaleLogarithmic): ScaleLogarithmic {
   const logScaleTickFormat = scale.tickFormat;
   scale.tickFormat = (count, specifier) => {
     return logScaleTickFormat(
@@ -101,3 +91,21 @@ export function scaleLog() {
   };
   return scale;
 }
+
+// Original d3.scale* should never be used; they won't support our custom tick formats.
+
+export function scaleLinear() {
+  return patchLinearishTickFormat(d3.scaleLinear());
+}
+
+export function scaleSymlog() {
+  return patchSymlogTickFormat(d3.scaleSymlog());
+}
+
+export function scalePow() {
+  return patchLinearishTickFormat(d3.scalePow());
+}
+
+export function scaleLog() {
+  return patchLogarithmicTickFormat(d3.scaleLog());
+}

packages/components/src/lib/draw/index.ts

@@ -109,6 +109,9 @@ export function drawAxes({
       }
 
       const text = xTickFormat(xTick);
+      if (text === "") {
+        continue; // we're probably rendering scaleLog, which has empty labels
+      }
       const { width: textWidth } = context.measureText(text);
       let startX = 0;
       if (i === 0) {
@@ -231,7 +234,7 @@ export function drawCursorLines({
     context.textAlign = "left";
     context.textBaseline = "bottom";
     const text = xLine.scale.tickFormat(
-      undefined,
+      Infinity, // important for scaleLog; https://github.com/d3/d3-scale/tree/main#log_tickFormat
       xLine.format
     )(xLine.scale.invert(point.x));
     const measured = context.measureText(text);
@@ -284,7 +287,7 @@ export function drawCursorLines({
     context.textAlign = "left";
     context.textBaseline = "bottom";
     const text = yLine.scale.tickFormat(
-      undefined,
+      Infinity, // important for scaleLog; https://github.com/d3/d3-scale/tree/main#log_tickFormat
       yLine.format
     )(yLine.scale.invert(point.y));
     const measured = context.measureText(text);

packages/squiggle-lang/__tests__/helpers/reducerHelpers.ts

@@ -59,10 +59,21 @@ export async function expectEvalToBe(code: string, answer: string) {
   expect(resultToString(await evaluateStringToResult(code))).toBe(answer);
 }
 
+export async function expectEvalToMatch(
+  code: string,
+  expected: string | RegExp
+) {
+  expect(resultToString(await evaluateStringToResult(code))).toMatch(expected);
+}
+
 export function testEvalToBe(expr: string, answer: string) {
   test(expr, async () => await expectEvalToBe(expr, answer));
 }
 
+export function testEvalToMatch(expr: string, expected: string | RegExp) {
+  test(expr, async () => await expectEvalToMatch(expr, expected));
+}
+
 export const MySkip = {
   testEvalToBe: (expr: string, answer: string) =>
     test.skip(expr, async () => await expectEvalToBe(expr, answer)),

packages/squiggle-lang/__tests__/library/scale_test.ts

@@ -0,0 +1,31 @@
+import { testEvalToBe, testEvalToMatch } from "../helpers/reducerHelpers.js";
+
+describe("Scales", () => {
+  testEvalToBe("Scale.linear()", "Linear scale");
+  testEvalToMatch(
+    "Scale.linear({ min: 10, max: 5 })",
+    "Max must be greater than min, got: min=10, max=5"
+  );
+
+  testEvalToBe("Scale.log()", "Logarithmic scale");
+  testEvalToMatch(
+    "Scale.log({ min: 10, max: 5 })",
+    "Max must be greater than min, got: min=10, max=5"
+  );
+  testEvalToMatch(
+    "Scale.log({ min: -1 })",
+    "Min must be over 0 for log scale, got: -1"
+  );
+
+  testEvalToBe("Scale.symlog()", "Symlog scale");
+  testEvalToMatch(
+    "Scale.symlog({ min: 10, max: 5 })",
+    "Max must be greater than min, got: min=10, max=5"
+  );
+
+  testEvalToBe("Scale.power({ exponent: 2 })", "Power scale (2)");
+  testEvalToMatch(
+    "Scale.power({ min: 10, max: 5, exponent: 2 })",
+    "Max must be greater than min, got: min=10, max=5"
+  );
+});

packages/squiggle-lang/src/fr/scale.ts

@@ -8,6 +8,7 @@ import {
 } from "../library/registry/frTypes.js";
 import { FnFactory } from "../library/registry/helpers.js";
 import { vScale } from "../value/index.js";
+import { REOther } from "../errors/messages.js";
 
 const maker = new FnFactory({
   nameSpace: "Scale",
@@ -20,13 +21,23 @@ const commonRecord = frRecord(
   ["tickFormat", frOptional(frString)]
 );
 
+function checkMinMax(min: number | null, max: number | null) {
+  if (min !== null && max !== null && max <= min) {
+    throw new REOther(
+      `Max must be greater than min, got: min=${min}, max=${max}`
+    );
+  }
+}
+
 export const library = [
   maker.make({
     name: "linear",
     output: "Scale",
     examples: [`Scale.linear({ min: 3, max: 10 })`],
     definitions: [
       makeDefinition([commonRecord], ([{ min, max, tickFormat }]) => {
+        checkMinMax(min, max);
+
         return vScale({
           type: "linear",
           min: min ?? undefined,
@@ -45,7 +56,11 @@ export const library = [
     examples: [`Scale.log({ min: 1, max: 100 })`],
     definitions: [
       makeDefinition([commonRecord], ([{ min, max, tickFormat }]) => {
-        // TODO - check that min > 0?
+        if (min !== null && min <= 0) {
+          throw new REOther(`Min must be over 0 for log scale, got: ${min}`);
+        }
+        checkMinMax(min, max);
+
         return vScale({
           type: "log",
           min: min ?? undefined,
@@ -64,6 +79,8 @@ export const library = [
     examples: [`Scale.symlog({ min: -10, max: 10 })`],
     definitions: [
       makeDefinition([commonRecord], ([{ min, max, tickFormat }]) => {
+        checkMinMax(min, max);
+
         return vScale({
           type: "symlog",
           min: min ?? undefined,
@@ -91,6 +108,8 @@ export const library = [
           ),
         ],
         ([{ min, max, tickFormat, exponent }]) => {
+          checkMinMax(min, max);
+
           return vScale({
             type: "power",
             min: min ?? undefined,