Merge pull request #172 from williaster/chris--proper-hist-bins

[histogram] squash end-inclusive histogram bins
williaster · Mar 13, 2019 · 6fd7b56 · 6fd7b56
2 parents a7abd58 + 6d8ee39
commit 6fd7b56
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/packages/demo/examples/02-histogram/renderHistogramTooltip.jsx b/packages/demo/examples/02-histogram/renderHistogramTooltip.jsx
@@ -8,7 +8,7 @@ export default function RenderTooltip({ datum, color }) {
     <div>
       <div>
         <strong style={{ color }}>
-          {typeof bin === 'undefined' ? `${bin0.toFixed(1)} to ${bin1.toFixed(1)}` : bin}
+          {typeof bin === 'undefined' ? `${bin0.toFixed(2)} to ${bin1.toFixed(2)}` : bin}
         </strong>
       </div>
       <br />

diff --git a/packages/histogram/src/utils/binNumericData.js b/packages/histogram/src/utils/binNumericData.js
@@ -20,14 +20,13 @@ const DEFAULT_BIN_COUNT = 10;
  */
 export default function binNumericData({
   allData,
-  binCount = DEFAULT_BIN_COUNT,
+  binCount: userBinCount = DEFAULT_BIN_COUNT,
   binValues,
   limits,
   rawDataByIndex,
   valueAccessor,
 }) {
-  let binThresholdCount = Math.max(...[2, binCount - 1]);
-  if (Array.isArray(binValues)) binThresholdCount = binValues.length;
+  const binCount = Array.isArray(binValues) ? binValues.length : userBinCount;
   const binsByIndex = {};
   const histogram = d3Histogram();
   let extent = d3Extent(allData, valueAccessor);
@@ -39,25 +38,36 @@ export default function binNumericData({
   }
   const scale = scaleLinear()
     .domain(extent)
-    .nice(binThresholdCount);
+    .nice(binCount);
 
-  histogram
-    .domain(limits || scale.domain())
-    .thresholds(binValues || scale.ticks(binThresholdCount));
+  histogram.domain(limits || scale.domain()).thresholds(binValues || scale.ticks(binCount));
 
   Object.keys(rawDataByIndex).forEach(index => {
     const data = rawDataByIndex[index];
     const seriesBins = histogram.value(valueAccessor)(data);
 
-    binsByIndex[index] = seriesBins.map((bin, i) => ({
+    // if the last bin equals the upper bound of the second to last bin, combine them
+    // see https://github.com/d3/d3-array/issues/46#issuecomment-269873644
+    const lastBinIndex = seriesBins.length - 1;
+    const lastBin = seriesBins[lastBinIndex];
+    const nextToLastBin = seriesBins[lastBinIndex - 1];
+    const shouldCombineEndBins =
+      nextToLastBin && nextToLastBin.x1 === lastBin.x0 && lastBin.x1 === lastBin.x0;
+    const filteredBins = shouldCombineEndBins ? seriesBins.slice(0, -1) : seriesBins;
+
+    binsByIndex[index] = filteredBins.map((bin, i) => ({
       bin0: bin.x0,
       // if the upper limit equals the lower one, use the delta between this bin and the last
       bin1:
         bin.x0 === bin.x1
           ? (i > 0 && bin.x0 + bin.x0 - seriesBins[i - 1].x0) || bin.x1 + 1
           : bin.x1,
-      data: bin,
-      count: bin.length,
+      data: [...bin].concat(
+        shouldCombineEndBins && (shouldCombineEndBins && i === lastBinIndex - 1 ? lastBin : []),
+      ),
+      // if the last bin was inclusive / omitted, add its count to the last bin
+      count:
+        bin.length + (shouldCombineEndBins && i === lastBinIndex - 1 ? lastBin.length || 0 : 0),
       id: i.toString(),
     }));
   });