Skip to content

Abbreviation for yellow in colorscale names is 'Yl' not 'YI' [fixes #269] #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 1, 2016
Merged

Abbreviation for yellow in colorscale names is 'Yl' not 'YI' [fixes #269] #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de10c1b
lint in colorscale lit
etpinard Feb 29, 2016
b2e1786
rename colorscales YIGnBu -> YlGnBu and YIOrRd -> YlOrRd :
etpinard Feb 29, 2016
da72f7b
lint (rm whitspaces in color strings)
etpinard Mar 1, 2016
0d0c297
update colorscale tests,
etpinard Mar 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 111 additions & 75 deletions src/components/colorscale/scales.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,84 +10,120 @@


module.exports = {
'Greys': [[0,'rgb(0,0,0)'],[1,'rgb(255,255,255)']],

'YIGnBu': [[0,'rgb(8, 29, 88)'],[0.125,'rgb(37, 52, 148)'],
[0.25,'rgb(34, 94, 168)'],[0.375,'rgb(29, 145, 192)'],
[0.5,'rgb(65, 182, 196)'],[0.625,'rgb(127, 205, 187)'],
[0.75,'rgb(199, 233, 180)'],[0.875,'rgb(237, 248, 217)'],
[1,'rgb(255, 255, 217)']],

'Greens': [[0,'rgb(0, 68, 27)'],[0.125,'rgb(0, 109, 44)'],
[0.25,'rgb(35, 139, 69)'],[0.375,'rgb(65, 171, 93)'],
[0.5,'rgb(116, 196, 118)'],[0.625,'rgb(161, 217, 155)'],
[0.75,'rgb(199, 233, 192)'],[0.875,'rgb(229, 245, 224)'],
[1,'rgb(247, 252, 245)']],

'YIOrRd': [[0,'rgb(128, 0, 38)'],[0.125,'rgb(189, 0, 38)'],
[0.25,'rgb(227, 26, 28)'],[0.375,'rgb(252, 78, 42)'],
[0.5,'rgb(253, 141, 60)'],[0.625,'rgb(254, 178, 76)'],
[0.75,'rgb(254, 217, 118)'],[0.875,'rgb(255, 237, 160)'],
[1,'rgb(255, 255, 204)']],

'Bluered': [[0,'rgb(0,0,255)'],[1,'rgb(255,0,0)']],
'Greys': [
[0, 'rgb(0,0,0)'], [1, 'rgb(255,255,255)']
],

'YlGnBu': [
[0, 'rgb(8,29,88)'], [0.125, 'rgb(37,52,148)'],
[0.25, 'rgb(34,94,168)'], [0.375, 'rgb(29,145,192)'],
[0.5, 'rgb(65,182,196)'], [0.625, 'rgb(127,205,187)'],
[0.75, 'rgb(199,233,180)'], [0.875, 'rgb(237,248,217)'],
[1, 'rgb(255,255,217)']
],

'Greens': [
[0, 'rgb(0,68,27)'], [0.125, 'rgb(0,109,44)'],
[0.25, 'rgb(35,139,69)'], [0.375, 'rgb(65,171,93)'],
[0.5, 'rgb(116,196,118)'], [0.625, 'rgb(161,217,155)'],
[0.75, 'rgb(199,233,192)'], [0.875, 'rgb(229,245,224)'],
[1, 'rgb(247,252,245)']
],

'YlOrRd': [
[0, 'rgb(128,0,38)'], [0.125, 'rgb(189,0,38)'],
[0.25, 'rgb(227,26,28)'], [0.375, 'rgb(252,78,42)'],
[0.5, 'rgb(253,141,60)'], [0.625, 'rgb(254,178,76)'],
[0.75, 'rgb(254,217,118)'], [0.875, 'rgb(255,237,160)'],
[1, 'rgb(255,255,204)']
],

'Bluered': [
[0, 'rgb(0,0,255)'], [1, 'rgb(255,0,0)']
],

// modified RdBu based on
// www.sandia.gov/~kmorel/documents/ColorMaps/ColorMapsExpanded.pdf
'RdBu': [[0,'rgb(5, 10, 172)'],[0.35,'rgb(106, 137, 247)'],
[0.5,'rgb(190,190,190)'],[0.6,'rgb(220, 170, 132)'],
[0.7,'rgb(230, 145, 90)'],[1,'rgb(178, 10, 28)']],
'RdBu': [
[0, 'rgb(5,10,172)'], [0.35, 'rgb(106,137,247)'],
[0.5, 'rgb(190,190,190)'], [0.6, 'rgb(220,170,132)'],
[0.7, 'rgb(230,145,90)'], [1, 'rgb(178,10,28)']
],

// Scale for non-negative numeric values
'Reds': [[0, 'rgb(220, 220, 220)'], [0.2,'rgb(245, 195, 157)'],
[0.4,'rgb(245, 160, 105)'], [1, 'rgb(178, 10, 28)']],
'Reds': [
[0, 'rgb(220,220,220)'], [0.2, 'rgb(245,195,157)'],
[0.4, 'rgb(245,160,105)'], [1, 'rgb(178,10,28)']
],

// Scale for non-positive numeric values
'Blues': [[0, 'rgb(5, 10, 172)'], [0.35, 'rgb(40, 60, 190)'],
[0.5, 'rgb(70, 100, 245)'], [0.6, 'rgb(90, 120, 245)'],
[0.7, 'rgb(106, 137, 247)'], [1, 'rgb(220, 220, 220)']],

'Picnic': [[0,'rgb(0,0,255)'],[0.1,'rgb(51,153,255)'],
[0.2,'rgb(102,204,255)'],[0.3,'rgb(153,204,255)'],
[0.4,'rgb(204,204,255)'],[0.5,'rgb(255,255,255)'],
[0.6,'rgb(255,204,255)'],[0.7,'rgb(255,153,255)'],
[0.8,'rgb(255,102,204)'],[0.9,'rgb(255,102,102)'],
[1,'rgb(255,0,0)']],

'Rainbow': [[0,'rgb(150,0,90)'],[0.125,'rgb(0, 0, 200)'],
[0.25,'rgb(0, 25, 255)'],[0.375,'rgb(0, 152, 255)'],
[0.5,'rgb(44, 255, 150)'],[0.625,'rgb(151, 255, 0)'],
[0.75,'rgb(255, 234, 0)'],[0.875,'rgb(255, 111, 0)'],
[1,'rgb(255, 0, 0)']],

'Portland': [[0,'rgb(12,51,131)'],[0.25,'rgb(10,136,186)'],
[0.5,'rgb(242,211,56)'],[0.75,'rgb(242,143,56)'],
[1,'rgb(217,30,30)']],

'Jet': [[0,'rgb(0,0,131)'],[0.125,'rgb(0,60,170)'],
[0.375,'rgb(5,255,255)'],[0.625,'rgb(255,255,0)'],
[0.875,'rgb(250,0,0)'],[1,'rgb(128,0,0)']],

'Hot': [[0,'rgb(0,0,0)'],[0.3,'rgb(230,0,0)'],
[0.6,'rgb(255,210,0)'],[1,'rgb(255,255,255)']],

'Blackbody': [[0,'rgb(0,0,0)'],[0.2,'rgb(230,0,0)'],
[0.4,'rgb(230,210,0)'],[0.7,'rgb(255,255,255)'],
[1,'rgb(160,200,255)']],

'Earth': [[0,'rgb(0,0,130)'],[0.1,'rgb(0,180,180)'],
[0.2,'rgb(40,210,40)'],[0.4,'rgb(230,230,50)'],
[0.6,'rgb(120,70,20)'],[1,'rgb(255,255,255)']],

'Electric': [[0,'rgb(0,0,0)'],[0.15,'rgb(30,0,100)'],
[0.4,'rgb(120,0,100)'],[0.6,'rgb(160,90,0)'],
[0.8,'rgb(230,200,0)'],[1,'rgb(255,250,220)']],

'Viridis': [[0,'#440154'],[0.06274509803921569,'#48186a'],
[0.12549019607843137,'#472d7b'],[0.18823529411764706,'#424086'],
[0.25098039215686274,'#3b528b'],[0.3137254901960784,'#33638d'],
[0.3764705882352941,'#2c728e'],[0.4392156862745098,'#26828e'],
[0.5019607843137255,'#21918c'],[0.5647058823529412,'#1fa088'],
[0.6274509803921569,'#28ae80'],[0.6901960784313725,'#3fbc73'],
[0.7529411764705882,'#5ec962'],[0.8156862745098039,'#84d44b'],
[0.8784313725490196,'#addc30'],[0.9411764705882353,'#d8e219'],
[1,'#fde725']]
'Blues': [
[0, 'rgb(5,10,172)'], [0.35, 'rgb(40,60,190)'],
[0.5, 'rgb(70,100,245)'], [0.6, 'rgb(90,120,245)'],
[0.7, 'rgb(106,137,247)'], [1, 'rgb(220,220,220)']
],

'Picnic': [
[0, 'rgb(0,0,255)'], [0.1, 'rgb(51,153,255)'],
[0.2, 'rgb(102,204,255)'], [0.3, 'rgb(153,204,255)'],
[0.4, 'rgb(204,204,255)'], [0.5, 'rgb(255,255,255)'],
[0.6, 'rgb(255,204,255)'], [0.7, 'rgb(255,153,255)'],
[0.8, 'rgb(255,102,204)'], [0.9, 'rgb(255,102,102)'],
[1, 'rgb(255,0,0)']
],

'Rainbow': [
[0, 'rgb(150,0,90)'], [0.125, 'rgb(0,0,200)'],
[0.25, 'rgb(0,25,255)'], [0.375, 'rgb(0,152,255)'],
[0.5, 'rgb(44,255,150)'], [0.625, 'rgb(151,255,0)'],
[0.75, 'rgb(255,234,0)'], [0.875, 'rgb(255,111,0)'],
[1, 'rgb(255,0,0)']
],

'Portland': [
[0, 'rgb(12,51,131)'], [0.25, 'rgb(10,136,186)'],
[0.5, 'rgb(242,211,56)'], [0.75, 'rgb(242,143,56)'],
[1, 'rgb(217,30,30)']
],

'Jet': [
[0, 'rgb(0,0,131)'], [0.125, 'rgb(0,60,170)'],
[0.375, 'rgb(5,255,255)'], [0.625, 'rgb(255,255,0)'],
[0.875, 'rgb(250,0,0)'], [1, 'rgb(128,0,0)']
],

'Hot': [
[0, 'rgb(0,0,0)'], [0.3, 'rgb(230,0,0)'],
[0.6, 'rgb(255,210,0)'], [1, 'rgb(255,255,255)']
],

'Blackbody': [
[0, 'rgb(0,0,0)'], [0.2, 'rgb(230,0,0)'],
[0.4, 'rgb(230,210,0)'], [0.7, 'rgb(255,255,255)'],
[1, 'rgb(160,200,255)']
],

'Earth': [
[0, 'rgb(0,0,130)'], [0.1, 'rgb(0,180,180)'],
[0.2, 'rgb(40,210,40)'], [0.4, 'rgb(230,230,50)'],
[0.6, 'rgb(120,70,20)'], [1, 'rgb(255,255,255)']
],

'Electric': [
[0, 'rgb(0,0,0)'], [0.15, 'rgb(30,0,100)'],
[0.4, 'rgb(120,0,100)'], [0.6, 'rgb(160,90,0)'],
[0.8, 'rgb(230,200,0)'], [1, 'rgb(255,250,220)']
],

'Viridis': [
[0, '#440154'], [0.06274509803921569, '#48186a'],
[0.12549019607843137, '#472d7b'], [0.18823529411764706, '#424086'],
[0.25098039215686274, '#3b528b'], [0.3137254901960784, '#33638d'],
[0.3764705882352941, '#2c728e'], [0.4392156862745098, '#26828e'],
[0.5019607843137255, '#21918c'], [0.5647058823529412, '#1fa088'],
[0.6274509803921569, '#28ae80'], [0.6901960784313725, '#3fbc73'],
[0.7529411764705882, '#5ec962'], [0.8156862745098039, '#84d44b'],
[0.8784313725490196, '#addc30'], [0.9411764705882353, '#d8e219'],
[1, '#fde725']
]
};
12 changes: 12 additions & 0 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ function cleanData(data, existingData) {

for(var tracei = 0; tracei < data.length; tracei++) {
var trace = data[tracei];

// assign uids to each trace and detect collisions.
if(!('uid' in trace) || suids.indexOf(trace.uid) !== -1) {
var newUid, i;
Expand Down Expand Up @@ -750,6 +751,17 @@ function cleanData(data, existingData) {
}
}

// fix typo in colorscale definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe put the check for YIGnBu/YIOrBu in a attribute coercion step instead of in logic code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want YIGnBu and YIOrBu to be considered valid colorscale names, we just want to clean up an old typo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... unless we want to consider YIGnBu an alias for YlGnBu.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, just seems hacky to have it being handled in plot_api.js instead of in the colorscale component - if something uses colorscale but doesn't flow through this codepath, it won't get the same treatment, whereas I'd assume that there's a common entry for colorscale related things in colorscale/____.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanData is pretty up-front about its purpose.

if(Plots.traceIs(trace, '2dMap')) {
if(trace.colorscale === 'YIGnBu') trace.colorscale = 'YlGnBu';
if(trace.colorscale === 'YIOrRd') trace.colorscale = 'YlOrRd';
}
if(Plots.traceIs(trace, 'markerColorscale') && trace.marker) {
var cont = trace.marker;
if(cont.colorscale === 'YIGnBu') cont.colorscale = 'YlGnBu';
if(cont.colorscale === 'YIOrRd') cont.colorscale = 'YlOrRd';
}

// prune empty containers made before the new nestedProperty
if(emptyContainer(trace, 'line')) delete trace.line;
if('marker' in trace) {
Expand Down
9 changes: 5 additions & 4 deletions test/jasmine/tests/colorscale_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ describe('Test colorscale:', function() {
z = [[0, -1.5], [-2, -10]];
calcColorscale(trace, z, '', 'z');
expect(trace.autocolorscale).toBe(true);
expect(trace.colorscale[5]).toEqual([1, 'rgb(220, 220, 220)']);
expect(trace.colorscale[5]).toEqual([1, 'rgb(220,220,220)']);
});

it('should be Blues when the only numerical z <= -0.5', function() {
Expand All @@ -338,7 +338,7 @@ describe('Test colorscale:', function() {
z = [[undefined, undefined], [-0.5, undefined]];
calcColorscale(trace, z, '', 'z');
expect(trace.autocolorscale).toBe(true);
expect(trace.colorscale[5]).toEqual([1, 'rgb(220, 220, 220)']);
expect(trace.colorscale[5]).toEqual([1, 'rgb(220,220,220)']);
});

it('should be Reds when the only numerical z >= 0.5', function() {
Expand All @@ -351,7 +351,7 @@ describe('Test colorscale:', function() {
z = [[undefined, undefined], [0.5, undefined]];
calcColorscale(trace, z, '', 'z');
expect(trace.autocolorscale).toBe(true);
expect(trace.colorscale[0]).toEqual([0, 'rgb(220, 220, 220)']);
expect(trace.colorscale[0]).toEqual([0, 'rgb(220,220,220)']);
});

it('should be reverse the auto scale when reversescale is true', function() {
Expand All @@ -365,7 +365,8 @@ describe('Test colorscale:', function() {
z = [[undefined, undefined], [0.5, undefined]];
calcColorscale(trace, z, '', 'z');
expect(trace.autocolorscale).toBe(true);
expect(trace.colorscale[trace.colorscale.length-1]).toEqual([1, 'rgb(220, 220, 220)']);
expect(trace.colorscale[trace.colorscale.length-1])
.toEqual([1, 'rgb(220,220,220)']);
});

});
Expand Down
53 changes: 53 additions & 0 deletions test/jasmine/tests/plot_api_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ var Bar = require('@src/traces/bar');
var Legend = require('@src/components/legend');
var pkg = require('../../../package.json');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');


describe('Test plot api', function() {
'use strict';
Expand Down Expand Up @@ -616,4 +619,54 @@ describe('Test plot api', function() {
expect(gd.data).toEqual(cachedData);
});
});

describe('cleanData', function() {
var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('should rename \'YIGnBu\' colorscales YlGnBu (2dMap case)', function() {
var data = [{
type: 'heatmap',
colorscale: 'YIGnBu'
}];

Plotly.plot(gd, data);
expect(gd.data[0].colorscale).toBe('YlGnBu');
});

it('should rename \'YIGnBu\' colorscales YlGnBu (markerColorscale case)', function() {
var data = [{
type: 'scattergeo',
marker: { colorscale: 'YIGnBu' }
}];

Plotly.plot(gd, data);
expect(gd.data[0].marker.colorscale).toBe('YlGnBu');
});

it('should rename \'YIOrRd\' colorscales YlOrRd (2dMap case)', function() {
var data = [{
type: 'contour',
colorscale: 'YIOrRd'
}];

Plotly.plot(gd, data);
expect(gd.data[0].colorscale).toBe('YlOrRd');
});

it('should rename \'YIOrRd\' colorscales YlOrRd (markerColorscale case)', function() {
var data = [{
type: 'scattergeo',
marker: { colorscale: 'YIOrRd' }
}];

Plotly.plot(gd, data);
expect(gd.data[0].marker.colorscale).toBe('YlOrRd');
});
});
});