fix: limit log scale domain

Limit the log scale limit and computed values to 1 or -1 depending on original domain. fix #21
elastic · Feb 15, 2019 · f7679a8 · f7679a8
1 parent 8359414
commit f7679a8
Show file tree

Hide file tree

Showing 6 changed files with 148 additions and 8 deletions.
diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts
@@ -1,7 +1,7 @@
 import { area, line } from 'd3-shape';
 import { DEFAULT_THEME } from '../themes/theme';
 import { SpecId } from '../utils/ids';
-import { Scale } from '../utils/scales/scales';
+import { Scale, ScaleType } from '../utils/scales/scales';
 import { CurveType, getCurveFactory } from './curves';
 import { LegendItem } from './legend';
 import { DataSeriesDatum } from './series';
@@ -99,11 +99,28 @@ export function renderBars(
   seriesKey: any[],
 ): BarGeometry[] {
   return dataset.map((datum, i) => {
+    const { x, y0, y1 } = datum;
+    let height = 0;
+    let y = 0;
+    if (yScale.type === ScaleType.Log) {
+      y = y1 === 0 ? yScale.range[0] : yScale.scale(y1);
+      let y0Scaled;
+      if (yScale.isInverted) {
+        y0Scaled = y0 === 0 ? yScale.range[1] : yScale.scale(y0);
+      } else {
+        y0Scaled = y0 === 0 ? yScale.range[0] : yScale.scale(y0);
+      }
+      height = y0Scaled - y;
+    } else {
+      y = yScale.scale(y1);
+      height = yScale.scale(y0) - y;
+    }
+
     return {
-      x: xScale.scale(datum.x) + xScale.bandwidth * orderIndex,
-      y: yScale.scale(datum.y1), // top most value
+      x: xScale.scale(x) + xScale.bandwidth * orderIndex,
+      y, // top most value
       width: xScale.bandwidth,
-      height: yScale.scale(datum.y0) - yScale.scale(datum.y1),
+      height,
       color,
       value: {
         specId,

diff --git a/src/lib/utils/scales/scale_band.ts b/src/lib/utils/scales/scale_band.ts
@@ -8,6 +8,7 @@ export class ScaleBand implements Scale {
   readonly type: ScaleType;
   readonly domain: any[];
   readonly range: number[];
+  readonly isInverted: boolean;
   private readonly d3Scale: any;
 
   constructor(
@@ -35,6 +36,7 @@ export class ScaleBand implements Scale {
     if (overrideBandwidth) {
       this.bandwidth = overrideBandwidth;
     }
+    this.isInverted = this.domain[0] > this.domain[1];
   }
 
   scale(value: any) {

diff --git a/src/lib/utils/scales/scale_continuous.ts b/src/lib/utils/scales/scale_continuous.ts
@@ -10,13 +10,64 @@ const SCALES = {
   [ScaleType.Time]: scaleTime,
 };
 
+export function limitToMin(value: number, positive: boolean) {
+  if (value === 0) {
+    return positive ? 1 : -1;
+  }
+  return value;
+}
+/**
+ * As log(0) = -Infinite, a log scale domain must be strictly-positive
+ * or strictly-negative; the domain must not include or cross zero value.
+ * We need to limit the domain scale to the right value on all possible cases.
+ * @param domain the domain to limit
+ */
+export function limitLogScaleDomain(domain: any[]) {
+  if (domain[0] === 0) {
+    if (domain[1] > 0) {
+      return [1, domain[1]];
+    } else if (domain[1] < 0) {
+      return [-1, domain[1]];
+    } else {
+      return [1, 1];
+    }
+  }
+  if (domain[1] === 0) {
+    if (domain[0] > 0) {
+      return [domain[0], 1];
+    } else if (domain[0] < 0) {
+      return [domain[0], -1];
+    } else {
+      return [1, 1];
+    }
+  }
+  if (domain[0] < 0 && domain[1] > 0) {
+    const isD0Min = Math.abs(domain[1]) - Math.abs(domain[0]) >= 0;
+    if (isD0Min) {
+      return [1, domain[1]];
+    } else {
+      return [domain[0], -1];
+    }
+  }
+  if (domain[0] > 0 && domain[1] < 0) {
+    const isD0Max = Math.abs(domain[0]) - Math.abs(domain[1]) >= 0;
+    if (isD0Max) {
+      return [domain[0], 1];
+    } else {
+      return [-1, domain[1]];
+    }
+  }
+  return domain;
+}
+
 export class ScaleContinuous implements Scale {
   readonly bandwidth: number;
   readonly minInterval: number;
   readonly step: number;
   readonly type: ScaleType;
   readonly domain: any[];
   readonly range: number[];
+  readonly isInverted: boolean;
   private readonly d3Scale: any;
 
   constructor(
@@ -28,16 +79,22 @@ export class ScaleContinuous implements Scale {
     minInterval?: number,
   ) {
     this.d3Scale = SCALES[type]();
-    this.d3Scale.domain(domain);
+    if (type === ScaleType.Log) {
+      this.domain = limitLogScaleDomain(domain);
+      this.d3Scale.domain(this.domain);
+    } else {
+      this.domain = domain;
+      this.d3Scale.domain(domain);
+    }
     this.d3Scale.range(range);
     this.d3Scale.clamp(clamp);
     // this.d3Scale.nice();
     this.bandwidth = bandwidth || 0;
     this.step = 0;
-    this.domain = domain;
     this.type = type;
     this.range = range;
     this.minInterval = minInterval || 0;
+    this.isInverted = this.domain[0] > this.domain[1];
   }
 
   scale(value: any) {

diff --git a/src/lib/utils/scales/scales.test.ts b/src/lib/utils/scales/scales.test.ts
@@ -1,3 +1,4 @@
+import { limitLogScaleDomain } from './scale_continuous';
 import { createContinuousScale, createOrdinalScale, ScaleType } from './scales';
 
 describe('Scale Test', () => {
@@ -49,6 +50,19 @@ describe('Scale Test', () => {
     const scaledValue3 = logScale.scale(5);
     expect(scaledValue3).toBe((Math.log(5) / Math.log(10)) * 100);
   });
+  test('Create an log scale starting with 0 as min', () => {
+    const data = [0, 10];
+    const minRange = 0;
+    const maxRange = 100;
+    const logScale = createContinuousScale(ScaleType.Log, data, minRange, maxRange);
+    const { domain, range } = logScale;
+    expect(domain).toEqual([1, 10]);
+    expect(range).toEqual([minRange, maxRange]);
+    const scaledValue1 = logScale.scale(1);
+    expect(scaledValue1).toBe(0);
+    const scaledValue3 = logScale.scale(5);
+    expect(scaledValue3).toBe((Math.log(5) / Math.log(10)) * 100);
+  });
   test('Create an sqrt scale', () => {
     const data = [0, 10];
     const minRange = 0;
@@ -62,4 +76,44 @@ describe('Scale Test', () => {
     const scaledValue3 = sqrtScale.scale(5);
     expect(scaledValue3).toBe((Math.sqrt(5) / Math.sqrt(10)) * 100);
   });
+  test('Check log scale domain limiting', () => {
+    let limitedDomain = limitLogScaleDomain([10, 20]);
+    expect(limitedDomain).toEqual([10, 20]);
+
+    limitedDomain = limitLogScaleDomain([0, 100]);
+    expect(limitedDomain).toEqual([1, 100]);
+
+    limitedDomain = limitLogScaleDomain([100, 0]);
+    expect(limitedDomain).toEqual([100, 1]);
+
+    limitedDomain = limitLogScaleDomain([0, 0]);
+    expect(limitedDomain).toEqual([1, 1]);
+
+    limitedDomain = limitLogScaleDomain([-100, 0]);
+    expect(limitedDomain).toEqual([-100, -1]);
+
+    limitedDomain = limitLogScaleDomain([0, -100]);
+    expect(limitedDomain).toEqual([-1, -100]);
+
+    limitedDomain = limitLogScaleDomain([-100, 100]);
+    expect(limitedDomain).toEqual([1, 100]);
+
+    limitedDomain = limitLogScaleDomain([-100, 50]);
+    expect(limitedDomain).toEqual([-100, -1]);
+
+    limitedDomain = limitLogScaleDomain([-100, 150]);
+    expect(limitedDomain).toEqual([1, 150]);
+
+    limitedDomain = limitLogScaleDomain([100, -100]);
+    expect(limitedDomain).toEqual([100, 1]);
+
+    limitedDomain = limitLogScaleDomain([100, -50]);
+    expect(limitedDomain).toEqual([100, 1]);
+
+    limitedDomain = limitLogScaleDomain([150, -100]);
+    expect(limitedDomain).toEqual([150, 1]);
+
+    limitedDomain = limitLogScaleDomain([50, -100]);
+    expect(limitedDomain).toEqual([-1, -100]);
+  });
 });
diff --git a/src/lib/utils/scales/scales.ts b/src/lib/utils/scales/scales.ts
@@ -9,6 +9,7 @@ export interface Scale {
   invert: (value: number) => any;
   bandwidth: number;
   type: ScaleType;
+  isInverted: boolean;
 }
 export type ScaleFunction = (value: any) => number;
 

diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx
@@ -153,7 +153,7 @@ storiesOf('Bar Chart', module)
       </Chart>
     );
   })
-  .add('with log y axis (TO FIX)', () => {
+  .add('with log y axis', () => {
     return (
       <Chart renderer="canvas" className={'story-chart'}>
         <Axis
@@ -175,7 +175,16 @@ storiesOf('Bar Chart', module)
           yScaleType={ScaleType.Log}
           xAccessor="x"
           yAccessors={['y']}
-          data={[{ x: 1, y: 2 }, { x: 2, y: 7 }, { x: 4, y: 3 }, { x: 9, y: 6 }]}
+          data={[
+            { x: 1, y: 0 },
+            { x: 2, y: 1 },
+            { x: 3, y: 2 },
+            { x: 4, y: 3 },
+            { x: 5, y: 4 },
+            { x: 6, y: 5 },
+            { x: 7, y: 6 },
+            { x: 8, y: 7 },
+          ]}
           yScaleToDataExtent={true}
         />
       </Chart>