From f7679a8141963f1940e904ccc71d6dcaee424d8e Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Mon, 4 Feb 2019 23:25:52 +0100 Subject: [PATCH] fix: limit log scale domain Limit the log scale limit and computed values to 1 or -1 depending on original domain. fix #21 --- src/lib/series/rendering.ts | 25 ++++++++-- src/lib/utils/scales/scale_band.ts | 2 + src/lib/utils/scales/scale_continuous.ts | 61 +++++++++++++++++++++++- src/lib/utils/scales/scales.test.ts | 54 +++++++++++++++++++++ src/lib/utils/scales/scales.ts | 1 + stories/bar_chart.tsx | 13 ++++- 6 files changed, 148 insertions(+), 8 deletions(-) diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index bd0e5e7cf1..2967fb132b 100644 --- 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 index 32d700b877..a79808b8ac 100644 --- 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 index 3b55576c36..d742b33108 100644 --- a/src/lib/utils/scales/scale_continuous.ts +++ b/src/lib/utils/scales/scale_continuous.ts @@ -10,6 +10,56 @@ 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; @@ -17,6 +67,7 @@ export class ScaleContinuous implements Scale { 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 index d6b3cbfba1..62b57ea88d 100644 --- 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 index dce3be9057..7875732aa4 100644 --- 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 index 0f264d3e67..ff7ab82894 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -153,7 +153,7 @@ storiesOf('Bar Chart', module) ); }) - .add('with log y axis (TO FIX)', () => { + .add('with log y axis', () => { return (