From a0714677a1b918548adf319536b4dff5dbf313d6 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 27 May 2020 13:01:04 -0700 Subject: [PATCH 1/4] feat: update functions --- packages/superset-ui-color/package.json | 4 +- .../superset-ui-color/src/SequentialScheme.ts | 67 ++++++++++++++++--- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/packages/superset-ui-color/package.json b/packages/superset-ui-color/package.json index dff510802d..00667aad82 100644 --- a/packages/superset-ui-color/package.json +++ b/packages/superset-ui-color/package.json @@ -27,7 +27,9 @@ }, "dependencies": { "@types/d3-scale": "^2.1.1", - "d3-scale": "^3.0.0" + "@types/d3-interpolate": "^1.3.1", + "d3-scale": "^3.0.0", + "d3-interpolate": "^1.4.0" }, "peerDependencies": { "@superset-ui/core": "^0.13.0" diff --git a/packages/superset-ui-color/src/SequentialScheme.ts b/packages/superset-ui-color/src/SequentialScheme.ts index a25b6f05af..afcbf61d8f 100644 --- a/packages/superset-ui-color/src/SequentialScheme.ts +++ b/packages/superset-ui-color/src/SequentialScheme.ts @@ -1,4 +1,5 @@ import { scaleLinear } from 'd3-scale'; +import { interpolateHcl } from 'd3-interpolate'; import ColorScheme, { ColorSchemeConfig } from './ColorScheme'; function range(count: number) { @@ -23,24 +24,68 @@ export default class SequentialScheme extends ColorScheme { this.isDiverging = isDiverging; } - createLinearScale(extent: number[] = [0, 1]) { - // Create matching domain - // because D3 continuous scale uses piecewise mapping - // between domain and range. - const valueScale = scaleLinear().range(extent); + /** + * Create a linear scale using the colors in this scheme as a range + * and interpolate domain [0, 1] + * to match the number of elements in the range + * e.g. + * If the range is ['red', 'white', 'blue'] + * the domain of this scale will be + * [0, 0.5, 1] + */ + private createPiecewiseScale() { const denominator = this.colors.length - 1; - const domain = range(this.colors.length).map(i => valueScale(i / denominator)); - return scaleLinear().domain(domain).range(this.colors).clamp(true); + return scaleLinear() + .domain(range(this.colors.length).map(i => i / denominator)) + .range(this.colors) + .interpolate(interpolateHcl) + .clamp(true); } - getColors(numColors: number = this.colors.length): string[] { + /** + * Return a linear scale with a new domain interpolated from the input domain + * to match the number of elements in the color scheme + * because D3 continuous scale uses piecewise mapping between domain and range. + * This is a common use-case when the domain is [min, max] + * and the palette such as ColorBrewer's has multiple elements. + * + * @param domain domain of the scale + * @param modifyRange Set this to true if you don't want to modify the domain and + * want to interpolate range to have the same number of elements with domain instead. + */ + createLinearScale(domain: number[] = [0, 1], modifyRange = false) { + if (modifyRange || domain.length === this.colors.length) { + return scaleLinear() + .interpolate(interpolateHcl) + .domain(domain) + .range(this.getColors(domain.length)); + } + + const piecewiseScale = this.createPiecewiseScale(); + const adjustDomain = scaleLinear().range(domain).clamp(true); + const newDomain = piecewiseScale.domain().map(d => adjustDomain(d)); + + return piecewiseScale.domain(newDomain); + } + + /** + * Get colors from this scheme + * @param numColors number of colors to return. + * Will interpolate the current scheme to match the number of colors requested + * @param extent The extent of the color range to use. + * For example [0.2, 1] will rescale the color scheme + * such that color values in the range [0, 0.2) are excluded from the scheme. + */ + getColors(numColors: number = this.colors.length, extent: number[] = [0, 1]): string[] { if (numColors === this.colors.length) { return this.colors; } - const colorScale = this.createLinearScale(); - const denominator = numColors - 1; - return range(numColors).map(i => colorScale(i / denominator)); + const piecewiseScale = this.createPiecewiseScale(); + const adjustExtent = scaleLinear().range(extent).clamp(true); + const denominator2 = numColors - 1; + + return range(numColors).map(i => piecewiseScale(adjustExtent(i / denominator2))); } } From 62382654f9cff55c8d6f38f338143506cc29d3b5 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 27 May 2020 13:29:56 -0700 Subject: [PATCH 2/4] test: add unit tests --- .../superset-ui-color/src/SequentialScheme.ts | 25 +++-- .../test/SequentialScheme.test.ts | 92 +++++++++++++------ 2 files changed, 77 insertions(+), 40 deletions(-) diff --git a/packages/superset-ui-color/src/SequentialScheme.ts b/packages/superset-ui-color/src/SequentialScheme.ts index afcbf61d8f..8bcab5b786 100644 --- a/packages/superset-ui-color/src/SequentialScheme.ts +++ b/packages/superset-ui-color/src/SequentialScheme.ts @@ -11,6 +11,11 @@ function range(count: number) { return values; } +function rangeZeroToOne(steps: number) { + const denominator = steps - 1; + return range(steps).map(i => i / denominator); +} + export interface SequentialSchemeConfig extends ColorSchemeConfig { isDiverging?: boolean; } @@ -33,11 +38,9 @@ export default class SequentialScheme extends ColorScheme { * the domain of this scale will be * [0, 0.5, 1] */ - private createPiecewiseScale() { - const denominator = this.colors.length - 1; - + private createZeroToOnePiecewiseScale() { return scaleLinear() - .domain(range(this.colors.length).map(i => i / denominator)) + .domain(rangeZeroToOne(this.colors.length)) .range(this.colors) .interpolate(interpolateHcl) .clamp(true); @@ -48,7 +51,7 @@ export default class SequentialScheme extends ColorScheme { * to match the number of elements in the color scheme * because D3 continuous scale uses piecewise mapping between domain and range. * This is a common use-case when the domain is [min, max] - * and the palette such as ColorBrewer's has multiple elements. + * and the palette has more than two colors. * * @param domain domain of the scale * @param modifyRange Set this to true if you don't want to modify the domain and @@ -62,8 +65,11 @@ export default class SequentialScheme extends ColorScheme { .range(this.getColors(domain.length)); } - const piecewiseScale = this.createPiecewiseScale(); - const adjustDomain = scaleLinear().range(domain).clamp(true); + const piecewiseScale = this.createZeroToOnePiecewiseScale(); + const adjustDomain = scaleLinear() + .domain(rangeZeroToOne(domain.length)) + .range(domain) + .clamp(true); const newDomain = piecewiseScale.domain().map(d => adjustDomain(d)); return piecewiseScale.domain(newDomain); @@ -82,10 +88,9 @@ export default class SequentialScheme extends ColorScheme { return this.colors; } - const piecewiseScale = this.createPiecewiseScale(); + const piecewiseScale = this.createZeroToOnePiecewiseScale(); const adjustExtent = scaleLinear().range(extent).clamp(true); - const denominator2 = numColors - 1; - return range(numColors).map(i => piecewiseScale(adjustExtent(i / denominator2))); + return rangeZeroToOne(numColors).map(x => piecewiseScale(adjustExtent(x))); } } diff --git a/packages/superset-ui-color/test/SequentialScheme.test.ts b/packages/superset-ui-color/test/SequentialScheme.test.ts index c05ab1dd8c..7b59a0e75d 100644 --- a/packages/superset-ui-color/test/SequentialScheme.test.ts +++ b/packages/superset-ui-color/test/SequentialScheme.test.ts @@ -17,41 +17,73 @@ describe('SequentialScheme', () => { expect(scheme2).toBeInstanceOf(SequentialScheme); }); }); - describe('.createLinearScale(extent)', () => { - it('returns a linear scale for the given extent', () => { + describe('.createLinearScale(domain, modifyRange)', () => { + it('returns a piecewise scale', () => { const scale = scheme.createLinearScale([10, 100]); - expect(scale(1)).toEqual('rgb(255, 255, 255)'); - expect(scale(10)).toEqual('rgb(255, 255, 255)'); - expect(scale(55)).toEqual('rgb(128, 128, 128)'); - expect(scale(100)).toEqual('rgb(0, 0, 0)'); - expect(scale(1000)).toEqual('rgb(0, 0, 0)'); + expect(scale.domain()).toHaveLength(scale.range().length); + const scale2 = scheme.createLinearScale([0, 10, 100]); + expect(scale2.domain()).toHaveLength(scale2.range().length); }); - it('uses [0, 1] as extent if not specified', () => { - const scale = scheme.createLinearScale(); - expect(scale(-1)).toEqual('rgb(255, 255, 255)'); - expect(scale(0)).toEqual('rgb(255, 255, 255)'); - expect(scale(0.5)).toEqual('rgb(128, 128, 128)'); - expect(scale(1)).toEqual('rgb(0, 0, 0)'); - expect(scale(2)).toEqual('rgb(0, 0, 0)'); + describe('domain', () => { + it('returns a linear scale for the given domain', () => { + const scale = scheme.createLinearScale([10, 100]); + expect(scale(1)).toEqual('rgb(255, 255, 255)'); + expect(scale(10)).toEqual('rgb(255, 255, 255)'); + expect(scale(55)).toEqual('rgb(128, 128, 128)'); + expect(scale(100)).toEqual('rgb(0, 0, 0)'); + expect(scale(1000)).toEqual('rgb(0, 0, 0)'); + }); + it('uses [0, 1] as domain if not specified', () => { + const scale = scheme.createLinearScale(); + expect(scale(-1)).toEqual('rgb(255, 255, 255)'); + expect(scale(0)).toEqual('rgb(255, 255, 255)'); + expect(scale(0.5)).toEqual('rgb(128, 128, 128)'); + expect(scale(1)).toEqual('rgb(0, 0, 0)'); + expect(scale(2)).toEqual('rgb(0, 0, 0)'); + }); + }); + describe('modifyRange', () => { + const scheme3 = new SequentialScheme({ + id: 'red to black', + colors: ['#ff0000', '#880000', '#000'], + }); + it('modifies domain by default', () => { + const scale = scheme3.createLinearScale([0, 100]); + expect(scale.domain()).toEqual([0, 50, 100]); + expect(scale.range()).toEqual(['#ff0000', '#880000', '#000']); + }); + it('modifies range instead of domain if set to true', () => { + const scale = scheme3.createLinearScale([0, 100], true); + expect(scale.domain()).toEqual([0, 100]); + expect(scale.range()).toEqual(['#ff0000', '#000']); + }); }); }); - describe('.getColors(numColors)', () => { - it('returns the original colors if numColors is not specified', () => { - expect(scheme.getColors()).toEqual(['#fff', '#000']); + describe('.getColors(numColors, extent)', () => { + describe('numColors', () => { + it('returns the original colors if numColors is not specified', () => { + expect(scheme.getColors()).toEqual(['#fff', '#000']); + }); + it('returns the exact number of colors if numColors is specified', () => { + expect(scheme.getColors(2)).toEqual(['#fff', '#000']); + expect(scheme.getColors(3)).toEqual([ + 'rgb(255, 255, 255)', + 'rgb(128, 128, 128)', + 'rgb(0, 0, 0)', + ]); + expect(scheme.getColors(4)).toEqual([ + 'rgb(255, 255, 255)', + 'rgb(170, 170, 170)', + 'rgb(85, 85, 85)', + 'rgb(0, 0, 0)', + ]); + }); }); - it('returns the exact number of colors if numColors is specified', () => { - expect(scheme.getColors(2)).toEqual(['#fff', '#000']); - expect(scheme.getColors(3)).toEqual([ - 'rgb(255, 255, 255)', - 'rgb(128, 128, 128)', - 'rgb(0, 0, 0)', - ]); - expect(scheme.getColors(4)).toEqual([ - 'rgb(255, 255, 255)', - 'rgb(170, 170, 170)', - 'rgb(85, 85, 85)', - 'rgb(0, 0, 0)', - ]); + describe('extent', () => { + it('adjust the range if extent is specified', () => { + expect(scheme.getColors(2, [0, 0.5])).toEqual(['rgb(255, 255, 255)', 'rgb(128, 128, 128)']); + expect(scheme.getColors(2, [0.5, 1])).toEqual(['rgb(128, 128, 128)', 'rgb(0, 0, 0)']); + }); }); }); }); From 55813e1c27d12f84c7f98d4738622b437d835cb3 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 27 May 2020 14:05:07 -0700 Subject: [PATCH 3/4] fix: unit tests --- .../superset-ui-color/src/SequentialScheme.ts | 2 +- .../test/SequentialScheme.test.ts | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/superset-ui-color/src/SequentialScheme.ts b/packages/superset-ui-color/src/SequentialScheme.ts index 8bcab5b786..b27be73cf1 100644 --- a/packages/superset-ui-color/src/SequentialScheme.ts +++ b/packages/superset-ui-color/src/SequentialScheme.ts @@ -84,7 +84,7 @@ export default class SequentialScheme extends ColorScheme { * such that color values in the range [0, 0.2) are excluded from the scheme. */ getColors(numColors: number = this.colors.length, extent: number[] = [0, 1]): string[] { - if (numColors === this.colors.length) { + if (numColors === this.colors.length && extent[0] === 0 && extent[1] === 1) { return this.colors; } diff --git a/packages/superset-ui-color/test/SequentialScheme.test.ts b/packages/superset-ui-color/test/SequentialScheme.test.ts index 7b59a0e75d..22f9d351c9 100644 --- a/packages/superset-ui-color/test/SequentialScheme.test.ts +++ b/packages/superset-ui-color/test/SequentialScheme.test.ts @@ -29,7 +29,7 @@ describe('SequentialScheme', () => { const scale = scheme.createLinearScale([10, 100]); expect(scale(1)).toEqual('rgb(255, 255, 255)'); expect(scale(10)).toEqual('rgb(255, 255, 255)'); - expect(scale(55)).toEqual('rgb(128, 128, 128)'); + expect(scale(55)).toEqual('rgb(119, 119, 119)'); expect(scale(100)).toEqual('rgb(0, 0, 0)'); expect(scale(1000)).toEqual('rgb(0, 0, 0)'); }); @@ -37,25 +37,25 @@ describe('SequentialScheme', () => { const scale = scheme.createLinearScale(); expect(scale(-1)).toEqual('rgb(255, 255, 255)'); expect(scale(0)).toEqual('rgb(255, 255, 255)'); - expect(scale(0.5)).toEqual('rgb(128, 128, 128)'); + expect(scale(0.5)).toEqual('rgb(119, 119, 119)'); expect(scale(1)).toEqual('rgb(0, 0, 0)'); expect(scale(2)).toEqual('rgb(0, 0, 0)'); }); }); describe('modifyRange', () => { const scheme3 = new SequentialScheme({ - id: 'red to black', - colors: ['#ff0000', '#880000', '#000'], + id: 'test-scheme3', + colors: ['#fee087', '#fa5c2e', '#800026'], }); it('modifies domain by default', () => { const scale = scheme3.createLinearScale([0, 100]); expect(scale.domain()).toEqual([0, 50, 100]); - expect(scale.range()).toEqual(['#ff0000', '#880000', '#000']); + expect(scale.range()).toEqual(['#fee087', '#fa5c2e', '#800026']); }); it('modifies range instead of domain if set to true', () => { const scale = scheme3.createLinearScale([0, 100], true); expect(scale.domain()).toEqual([0, 100]); - expect(scale.range()).toEqual(['#ff0000', '#000']); + expect(scale.range()).toEqual(['rgb(254, 224, 135)', 'rgb(128, 0, 38)']); }); }); }); @@ -68,21 +68,21 @@ describe('SequentialScheme', () => { expect(scheme.getColors(2)).toEqual(['#fff', '#000']); expect(scheme.getColors(3)).toEqual([ 'rgb(255, 255, 255)', - 'rgb(128, 128, 128)', + 'rgb(119, 119, 119)', 'rgb(0, 0, 0)', ]); expect(scheme.getColors(4)).toEqual([ 'rgb(255, 255, 255)', - 'rgb(170, 170, 170)', - 'rgb(85, 85, 85)', + 'rgb(162, 162, 162)', + 'rgb(78, 78, 78)', 'rgb(0, 0, 0)', ]); }); }); describe('extent', () => { it('adjust the range if extent is specified', () => { - expect(scheme.getColors(2, [0, 0.5])).toEqual(['rgb(255, 255, 255)', 'rgb(128, 128, 128)']); - expect(scheme.getColors(2, [0.5, 1])).toEqual(['rgb(128, 128, 128)', 'rgb(0, 0, 0)']); + expect(scheme.getColors(2, [0, 0.5])).toEqual(['rgb(255, 255, 255)', 'rgb(119, 119, 119)']); + expect(scheme.getColors(2, [0.5, 1])).toEqual(['rgb(119, 119, 119)', 'rgb(0, 0, 0)']); }); }); }); From 2786b28052f96c4191bb59cf3357abc717b88366 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Thu, 28 May 2020 14:40:28 -0700 Subject: [PATCH 4/4] fix: address comments and use piecewise --- .../superset-ui-color/src/SequentialScheme.ts | 57 +++---------------- 1 file changed, 9 insertions(+), 48 deletions(-) diff --git a/packages/superset-ui-color/src/SequentialScheme.ts b/packages/superset-ui-color/src/SequentialScheme.ts index b27be73cf1..8b04d31d80 100644 --- a/packages/superset-ui-color/src/SequentialScheme.ts +++ b/packages/superset-ui-color/src/SequentialScheme.ts @@ -1,21 +1,7 @@ import { scaleLinear } from 'd3-scale'; -import { interpolateHcl } from 'd3-interpolate'; +import { interpolateHcl, interpolateNumber, piecewise, quantize } from 'd3-interpolate'; import ColorScheme, { ColorSchemeConfig } from './ColorScheme'; -function range(count: number) { - const values: number[] = []; - for (let i = 0; i < count; i += 1) { - values.push(i); - } - - return values; -} - -function rangeZeroToOne(steps: number) { - const denominator = steps - 1; - return range(steps).map(i => i / denominator); -} - export interface SequentialSchemeConfig extends ColorSchemeConfig { isDiverging?: boolean; } @@ -29,23 +15,6 @@ export default class SequentialScheme extends ColorScheme { this.isDiverging = isDiverging; } - /** - * Create a linear scale using the colors in this scheme as a range - * and interpolate domain [0, 1] - * to match the number of elements in the range - * e.g. - * If the range is ['red', 'white', 'blue'] - * the domain of this scale will be - * [0, 0.5, 1] - */ - private createZeroToOnePiecewiseScale() { - return scaleLinear() - .domain(rangeZeroToOne(this.colors.length)) - .range(this.colors) - .interpolate(interpolateHcl) - .clamp(true); - } - /** * Return a linear scale with a new domain interpolated from the input domain * to match the number of elements in the color scheme @@ -58,21 +27,13 @@ export default class SequentialScheme extends ColorScheme { * want to interpolate range to have the same number of elements with domain instead. */ createLinearScale(domain: number[] = [0, 1], modifyRange = false) { - if (modifyRange || domain.length === this.colors.length) { - return scaleLinear() - .interpolate(interpolateHcl) - .domain(domain) - .range(this.getColors(domain.length)); - } - - const piecewiseScale = this.createZeroToOnePiecewiseScale(); - const adjustDomain = scaleLinear() - .domain(rangeZeroToOne(domain.length)) - .range(domain) - .clamp(true); - const newDomain = piecewiseScale.domain().map(d => adjustDomain(d)); + const scale = scaleLinear().interpolate(interpolateHcl).clamp(true); - return piecewiseScale.domain(newDomain); + return modifyRange || domain.length === this.colors.length + ? scale.domain(domain).range(this.getColors(domain.length)) + : scale + .domain(quantize(piecewise(interpolateNumber, domain), this.colors.length)) + .range(this.colors); } /** @@ -88,9 +49,9 @@ export default class SequentialScheme extends ColorScheme { return this.colors; } - const piecewiseScale = this.createZeroToOnePiecewiseScale(); + const piecewiseScale: (t: number) => string = piecewise(interpolateHcl, this.colors); const adjustExtent = scaleLinear().range(extent).clamp(true); - return rangeZeroToOne(numColors).map(x => piecewiseScale(adjustExtent(x))); + return quantize(t => piecewiseScale(adjustExtent(t)), numColors); } }