fix quantileIndex (#276)

* fix quantileIndex * don’t invoke accessor more than once
d3 · May 30, 2023 · f005cf8 · f005cf8
1 parent c62f825
commit f005cf8
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 15 deletions.
diff --git a/src/quantile.js b/src/quantile.js
@@ -32,14 +32,16 @@ export function quantileSorted(values, p, valueof = number) {
   return value0 + (value1 - value0) * (i - i0);
 }
 
-export function quantileIndex(values, p, valueof) {
-  values = Float64Array.from(numbers(values, valueof));
-  if (!(n = values.length) || isNaN(p = +p)) return;
-  if (p <= 0 || n < 2) return minIndex(values);
-  if (p >= 1) return maxIndex(values);
-  var n,
-      i = Math.floor((n - 1) * p),
-      order = (i, j) => ascendingDefined(values[i], values[j]),
-      index = quickselect(Uint32Array.from(values, (_, i) => i), i, 0, n - 1, order);
-  return greatest(index.subarray(0, i + 1), i => values[i]);
+export function quantileIndex(values, p, valueof = number) {
+  if (isNaN(p = +p)) return;
+  numbers = Float64Array.from(values, (_, i) => number(valueof(values[i], i, values)));
+  if (p <= 0) return minIndex(numbers);
+  if (p >= 1) return maxIndex(numbers);
+  var numbers,
+      index = Uint32Array.from(values, (_, i) => i),
+      j = numbers.length - 1,
+      i = Math.floor(j * p);
+  quickselect(index, i, 0, j, (i, j) => ascendingDefined(numbers[i], numbers[j]));
+  i = greatest(index.subarray(0, i + 1), (i) => numbers[i]);
+  return i >= 0 ? i : -1;
 }
diff --git a/test/median-test.js b/test/median-test.js
@@ -104,7 +104,7 @@ it("medianIndex(array) returns the index", () => {
   assert.deepStrictEqual(medianIndex([1, 3, 2]), 2);
   assert.deepStrictEqual(medianIndex([2, 3, 1]), 0);
   assert.deepStrictEqual(medianIndex([1]), 0);
-  assert.deepStrictEqual(medianIndex([]), undefined);
+  assert.deepStrictEqual(medianIndex([]), -1);
 });
 
 

diff --git a/test/quantile-test.js b/test/quantile-test.js
@@ -96,7 +96,7 @@ it("quantileIndex(array, p) returns the index", () => {
   assert.deepStrictEqual(quantileIndex([1, 3, 2], 0.2), 0);
   assert.deepStrictEqual(quantileIndex([2, 3, 1], 0.2), 2);
   assert.deepStrictEqual(quantileIndex([1], 0.2), 0);
-  assert.deepStrictEqual(quantileIndex([], 0.2), undefined);
+  assert.deepStrictEqual(quantileIndex([], 0.2), -1);
 });
 
 it("quantileIndex(array, 0) returns the minimum index", () => {
@@ -105,16 +105,25 @@ it("quantileIndex(array, 0) returns the minimum index", () => {
   assert.deepStrictEqual(quantileIndex([1, 3, 2], 0), 0);
   assert.deepStrictEqual(quantileIndex([2, 3, 1], 0), 2);
   assert.deepStrictEqual(quantileIndex([1], 0), 0);
-  assert.deepStrictEqual(quantileIndex([], 0), undefined);
+  assert.deepStrictEqual(quantileIndex([], 0), -1);
 });
 
-it("quantileIndex(array, 1) returns the maxium index", () => {
+it("quantileIndex(array, 1) returns the maximum index", () => {
   assert.deepStrictEqual(quantileIndex([1, 2], 1), 1);
   assert.deepStrictEqual(quantileIndex([1, 2, 3], 1), 2);
   assert.deepStrictEqual(quantileIndex([1, 3, 2], 1), 1);
   assert.deepStrictEqual(quantileIndex([2, 3, 1], 1), 1);
   assert.deepStrictEqual(quantileIndex([1], 1), 0);
-  assert.deepStrictEqual(quantileIndex([], 1), undefined);
+  assert.deepStrictEqual(quantileIndex([], 1), -1);
+});
+
+it("quantileIndex(array, 0.5) handles undefined values", () => {
+  assert.deepStrictEqual(quantileIndex([1, 1, 1, null, 2, 3, 3, 3], 0.5), 4);
+  assert.deepStrictEqual(quantileIndex([1, 1, 1, null, 2, 3, 3, 3], 0.5, (d) => d), 4);
+});
+
+it("quantileIndex(array, 0.5) returns the first of equivalent values", () => {
+  assert.deepStrictEqual(quantileIndex([1, 1, 1, 2, 2, 3, 3, 3], 0.5), 4);
 });
 
 function box(value) {