Skip to content

Commit

Permalink
Improve categorical color management (apache#5815)
Browse files Browse the repository at this point in the history
* Create new classes for handling categorical colors

* verify to pass existing unit tests

* separate logic for forcing color and getting color

* replace getColorFromScheme with CategoricalColorManager

* organize static functions

* migrate to new function

* Remove ALL_COLOR_SCHEMES

* move sequential colors to another file

* extract categorical colors to separate file

* move airbnb and lyft colors to separate files

* fix missing toFunction()

* Rewrite to support local and global force items, plus namespacing.

* fix references

* revert nvd3

* update namespace api

* Update the visualizations

* update usage with static functions

* update unit test

* add unit test

* rename default namespace

* add unit test for color namespace

* add unit test for namespace

* start unit test for colorschememanager

* add unit tests for color scheme manager

* check returns for chaining

* complete unit test for the new classes

* fix color tests

* update unit tests

* update unit tests

* move color scheme registration to common

* update unit test

* rename sharedForcedColors to parentForcedColors

* remove import
  • Loading branch information
kristw authored and williaster committed Sep 12, 2018
1 parent 1b894d7 commit 72cf576
Show file tree
Hide file tree
Showing 26 changed files with 1,186 additions and 595 deletions.
1 change: 1 addition & 0 deletions superset/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"test": "spec"
},
"scripts": {
"tdd": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*' --watch --recursive",
"test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*'",
"test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js",
"cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/shim.js --require ignore-styles 'spec/**/*_spec.*'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { Creatable } from 'react-select';

import ColorSchemeControl from
'../../../../src/explore/components/controls/ColorSchemeControl';
import { ALL_COLOR_SCHEMES } from '../../../../src/modules/colors';
import { getAllSchemes } from '../../../../src/modules/ColorSchemeManager';

const defaultProps = {
options: Object.keys(ALL_COLOR_SCHEMES).map(s => ([s, s])),
options: Object.keys(getAllSchemes()).map(s => ([s, s])),
};

describe('ColorSchemeControl', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { it, describe, before } from 'mocha';
import { expect } from 'chai';
import CategoricalColorNamespace, {
getNamespace,
getScale,
getColor,
DEFAULT_NAMESPACE,
} from '../../../src/modules/CategoricalColorNamespace';
import { registerScheme } from '../../../src/modules/ColorSchemeManager';

describe('CategoricalColorNamespace', () => {
before(() => {
registerScheme('testColors', ['red', 'green', 'blue']);
registerScheme('testColors2', ['red', 'green', 'blue']);
});
it('The class constructor cannot be accessed directly', () => {
expect(CategoricalColorNamespace).to.not.be.a('Function');
});
describe('static getNamespace()', () => {
it('returns default namespace if name is not specified', () => {
const namespace = getNamespace();
expect(namespace !== undefined).to.equal(true);
expect(namespace.name).to.equal(DEFAULT_NAMESPACE);
});
it('returns namespace with specified name', () => {
const namespace = getNamespace('myNamespace');
expect(namespace !== undefined).to.equal(true);
expect(namespace.name).to.equal('myNamespace');
});
it('returns existing instance if the name already exists', () => {
const ns1 = getNamespace('myNamespace');
const ns2 = getNamespace('myNamespace');
expect(ns1).to.equal(ns2);
const ns3 = getNamespace();
const ns4 = getNamespace();
expect(ns3).to.equal(ns4);
});
});
describe('.getScale()', () => {
it('returns a CategoricalColorScale from given scheme name', () => {
const namespace = getNamespace('test-get-scale1');
const scale = namespace.getScale('testColors');
expect(scale).to.not.equal(undefined);
expect(scale.getColor('dog')).to.not.equal(undefined);
});
it('returns same scale if the scale with that name already exists in this namespace', () => {
const namespace = getNamespace('test-get-scale2');
const scale1 = namespace.getScale('testColors');
const scale2 = namespace.getScale('testColors2');
const scale3 = namespace.getScale('testColors2');
const scale4 = namespace.getScale('testColors');
expect(scale1).to.equal(scale4);
expect(scale2).to.equal(scale3);
});
});
describe('.setColor()', () => {
it('overwrites color for all CategoricalColorScales in this namespace', () => {
const namespace = getNamespace('test-set-scale1');
namespace.setColor('dog', 'black');
const scale = namespace.getScale('testColors');
expect(scale.getColor('dog')).to.equal('black');
expect(scale.getColor('boy')).to.not.equal('black');
});
it('can override forcedColors in each scale', () => {
const namespace = getNamespace('test-set-scale2');
namespace.setColor('dog', 'black');
const scale = namespace.getScale('testColors');
scale.setColor('dog', 'pink');
expect(scale.getColor('dog')).to.equal('black');
expect(scale.getColor('boy')).to.not.equal('black');
});
it('does not affect scales in other namespaces', () => {
const ns1 = getNamespace('test-set-scale3.1');
ns1.setColor('dog', 'black');
const scale1 = ns1.getScale('testColors');
const ns2 = getNamespace('test-set-scale3.2');
const scale2 = ns2.getScale('testColors');
expect(scale1.getColor('dog')).to.equal('black');
expect(scale2.getColor('dog')).to.not.equal('black');
});
it('returns the namespace instance', () => {
const ns1 = getNamespace('test-set-scale3.1');
const ns2 = ns1.setColor('dog', 'black');
expect(ns1).to.equal(ns2);
});
});
describe('static getScale()', () => {
it('getScale() returns a CategoricalColorScale with default scheme in default namespace', () => {
const scale = getScale();
expect(scale).to.not.equal(undefined);
const scale2 = getNamespace().getScale();
expect(scale).to.equal(scale2);
});
it('getScale(scheme) returns a CategoricalColorScale with specified scheme in default namespace', () => {
const scale = getScale('testColors');
expect(scale).to.not.equal(undefined);
const scale2 = getNamespace().getScale('testColors');
expect(scale).to.equal(scale2);
});
it('getScale(scheme, namespace) returns a CategoricalColorScale with specified scheme in specified namespace', () => {
const scale = getScale('testColors', 'test-getScale');
expect(scale).to.not.equal(undefined);
const scale2 = getNamespace('test-getScale').getScale('testColors');
expect(scale).to.equal(scale2);
});
});
describe('static getColor()', () => {
it('getColor(value) returns a color from default scheme in default namespace', () => {
const value = 'dog';
const color = getColor(value);
const color2 = getNamespace().getScale().getColor(value);
expect(color).to.equal(color2);
});
it('getColor(value, scheme) returns a color from specified scheme in default namespace', () => {
const value = 'dog';
const scheme = 'testColors';
const color = getColor(value, scheme);
const color2 = getNamespace().getScale(scheme).getColor(value);
expect(color).to.equal(color2);
});
it('getColor(value, scheme, namespace) returns a color from specified scheme in specified namespace', () => {
const value = 'dog';
const scheme = 'testColors';
const namespace = 'test-getColor';
const color = getColor(value, scheme, namespace);
const color2 = getNamespace(namespace).getScale(scheme).getColor(value);
expect(color).to.equal(color2);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { it, describe } from 'mocha';
import { expect } from 'chai';
import CategoricalColorScale from '../../../src/modules/CategoricalColorScale';

describe('CategoricalColorScale', () => {
it('exists', () => {
expect(CategoricalColorScale !== undefined).to.equal(true);
});

describe('new CategoricalColorScale(colors, parentForcedColors)', () => {
it('can create new scale when parentForcedColors is not given', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
expect(scale).to.be.instanceOf(CategoricalColorScale);
});
it('can create new scale when parentForcedColors is given', () => {
const parentForcedColors = {};
const scale = new CategoricalColorScale(['blue', 'red', 'green'], parentForcedColors);
expect(scale).to.be.instanceOf(CategoricalColorScale);
expect(scale.parentForcedColors).to.equal(parentForcedColors);
});
});
describe('.getColor(value)', () => {
it('returns same color for same value', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const c1 = scale.getColor('pig');
const c2 = scale.getColor('horse');
const c3 = scale.getColor('pig');
scale.getColor('cow');
const c5 = scale.getColor('horse');

expect(c1).to.equal(c3);
expect(c2).to.equal(c5);
});
it('returns different color for consecutive items', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const c1 = scale.getColor('pig');
const c2 = scale.getColor('horse');
const c3 = scale.getColor('cat');

expect(c1).to.not.equal(c2);
expect(c2).to.not.equal(c3);
expect(c3).to.not.equal(c1);
});
it('recycles colors when number of items exceed available colors', () => {
const colorSet = {};
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const colors = [
scale.getColor('pig'),
scale.getColor('horse'),
scale.getColor('cat'),
scale.getColor('cow'),
scale.getColor('donkey'),
scale.getColor('goat'),
];
colors.forEach((color) => {
if (colorSet[color]) {
colorSet[color]++;
} else {
colorSet[color] = 1;
}
});
expect(Object.keys(colorSet).length).to.equal(3);
['blue', 'red', 'green'].forEach((color) => {
expect(colorSet[color]).to.equal(2);
});
});
});
describe('.setColor(value, forcedColor)', () => {
it('overrides default color', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
scale.setColor('pig', 'pink');
expect(scale.getColor('pig')).to.equal('pink');
});
it('does not override parentForcedColors', () => {
const scale1 = new CategoricalColorScale(['blue', 'red', 'green']);
scale1.setColor('pig', 'black');
const scale2 = new CategoricalColorScale(['blue', 'red', 'green'], scale1.forcedColors);
scale2.setColor('pig', 'pink');
expect(scale1.getColor('pig')).to.equal('black');
expect(scale2.getColor('pig')).to.equal('black');
});
it('returns the scale', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const output = scale.setColor('pig', 'pink');
expect(scale).to.equal(output);
});
});
describe('.toFunction()', () => {
it('returns a function that wraps getColor', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const colorFn = scale.toFunction();
expect(scale.getColor('pig')).to.equal(colorFn('pig'));
expect(scale.getColor('cat')).to.equal(colorFn('cat'));
});
});
});
141 changes: 141 additions & 0 deletions superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { it, describe, beforeEach } from 'mocha';
import { expect } from 'chai';
import ColorSchemeManager, {
getInstance,
getScheme,
getAllSchemes,
getDefaultSchemeName,
setDefaultSchemeName,
registerScheme,
registerMultipleSchemes,
} from '../../../src/modules/ColorSchemeManager';

describe('ColorSchemeManager', () => {
beforeEach(() => {
const m = getInstance();
m.clearScheme();
m.registerScheme('test', ['red', 'green', 'blue']);
m.registerScheme('test2', ['orange', 'yellow', 'pink']);
m.setDefaultSchemeName('test');
});
it('The class constructor cannot be accessed directly', () => {
expect(ColorSchemeManager).to.not.be.a('Function');
});
describe('static getInstance()', () => {
it('returns a singleton instance', () => {
const m1 = getInstance();
const m2 = getInstance();
expect(m1).to.not.equal(undefined);
expect(m1).to.equal(m2);
});
});
describe('.getScheme()', () => {
it('.getScheme() returns default color scheme', () => {
const scheme = getInstance().getScheme();
expect(scheme).to.deep.equal(['red', 'green', 'blue']);
});
it('.getScheme(name) returns color scheme with specified name', () => {
const scheme = getInstance().getScheme('test2');
expect(scheme).to.deep.equal(['orange', 'yellow', 'pink']);
});
});
describe('.getAllSchemes()', () => {
it('returns all registered schemes', () => {
const schemes = getInstance().getAllSchemes();
expect(schemes).to.deep.equal({
test: ['red', 'green', 'blue'],
test2: ['orange', 'yellow', 'pink'],
});
});
});
describe('.getDefaultSchemeName()', () => {
it('returns default scheme name', () => {
const name = getInstance().getDefaultSchemeName();
expect(name).to.equal('test');
});
});
describe('.setDefaultSchemeName()', () => {
it('set default scheme name', () => {
getInstance().setDefaultSchemeName('test2');
const name = getInstance().getDefaultSchemeName();
expect(name).to.equal('test2');
getInstance().setDefaultSchemeName('test');
});
it('returns the ColorSchemeManager instance', () => {
const instance = getInstance().setDefaultSchemeName('test');
expect(instance).to.equal(getInstance());
});
});
describe('.registerScheme(name, colors)', () => {
it('sets schemename and color', () => {
getInstance().registerScheme('test3', ['cyan', 'magenta']);
const scheme = getInstance().getScheme('test3');
expect(scheme).to.deep.equal(['cyan', 'magenta']);
});
it('returns the ColorSchemeManager instance', () => {
const instance = getInstance().registerScheme('test3', ['cyan', 'magenta']);
expect(instance).to.equal(getInstance());
});
});
describe('.registerMultipleSchemes(object)', () => {
it('sets multiple schemes at once', () => {
getInstance().registerMultipleSchemes({
test4: ['cyan', 'magenta'],
test5: ['brown', 'purple'],
});
const scheme1 = getInstance().getScheme('test4');
expect(scheme1).to.deep.equal(['cyan', 'magenta']);
const scheme2 = getInstance().getScheme('test5');
expect(scheme2).to.deep.equal(['brown', 'purple']);
});
it('returns the ColorSchemeManager instance', () => {
const instance = getInstance().registerMultipleSchemes({
test4: ['cyan', 'magenta'],
test5: ['brown', 'purple'],
});
expect(instance).to.equal(getInstance());
});
});
describe('static getScheme()', () => {
it('is equivalent to getInstance().getScheme()', () => {
expect(getInstance().getScheme()).to.equal(getScheme());
});
});
describe('static getAllSchemes()', () => {
it('is equivalent to getInstance().getAllSchemes()', () => {
expect(getInstance().getAllSchemes()).to.equal(getAllSchemes());
});
});
describe('static getDefaultSchemeName()', () => {
it('is equivalent to getInstance().getDefaultSchemeName()', () => {
expect(getInstance().getDefaultSchemeName()).to.equal(getDefaultSchemeName());
});
});
describe('static setDefaultSchemeName()', () => {
it('is equivalent to getInstance().setDefaultSchemeName()', () => {
setDefaultSchemeName('test2');
const name = getInstance().getDefaultSchemeName();
expect(name).to.equal('test2');
setDefaultSchemeName('test');
});
});
describe('static registerScheme()', () => {
it('is equivalent to getInstance().registerScheme()', () => {
registerScheme('test3', ['cyan', 'magenta']);
const scheme = getInstance().getScheme('test3');
expect(scheme).to.deep.equal(['cyan', 'magenta']);
});
});
describe('static registerMultipleSchemes()', () => {
it('is equivalent to getInstance().registerMultipleSchemes()', () => {
registerMultipleSchemes({
test4: ['cyan', 'magenta'],
test5: ['brown', 'purple'],
});
const scheme1 = getInstance().getScheme('test4');
expect(scheme1).to.deep.equal(['cyan', 'magenta']);
const scheme2 = getInstance().getScheme('test5');
expect(scheme2).to.deep.equal(['brown', 'purple']);
});
});
});
Loading

0 comments on commit 72cf576

Please sign in to comment.