Skip to content
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

Jump to bottom

initial implementation of layout.colorscale #3274

Merged
merged 15 commits into from
Nov 23, 2018
Merged

initial implementation of layout.colorscale #3274

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6b1b940
initial implementation of `layout.colorscale`
antoinerg Nov 20, 2018
0e98c5b
fix editType
antoinerg Nov 20, 2018
6e61f3b
fix parcoords, fix default colorscale
antoinerg Nov 20, 2018
f5bcb71
do not coerce layout.colorscale if not specified
antoinerg Nov 20, 2018
a299ce0
move layout colorscale attributes into its own file
antoinerg Nov 21, 2018
2a20414
update description for layout.colorscale attributes
antoinerg Nov 21, 2018
2460fc3
fix lint
antoinerg Nov 21, 2018
0c13592
change colorscaleCalc's signature, cleanup code
antoinerg Nov 21, 2018
93ce241
update colorscale tests to provide a layout object to calcColorscale
antoinerg Nov 21, 2018
d263a7f
fix lint
antoinerg Nov 21, 2018
84f0ea8
revert parcoords_tests.js to master
antoinerg Nov 21, 2018
ee222d5
test layout.colorscale inheritance
antoinerg Nov 22, 2018
75e8910
improve code readability
antoinerg Nov 22, 2018
e50a6aa
add a mock using template and layout.colorscale
antoinerg Nov 22, 2018
dca4350
set autocolorscale: true in template to properly test mock
antoinerg Nov 23, 2018
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
6 changes: 3 additions & 3 deletions src/components/colorscale/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ module.exports = function calc(trace, vals, containerStr, cLetter) {
doUpdate(autoAttr, (auto !== false || (min === undefined && max === undefined)));

if(container.autocolorscale) {
if(min * max < 0) scl = scales.RdBu;
else if(min >= 0) scl = scales.Reds;
else scl = scales.Blues;
if(min * max < 0) scl = container.diverging || scales.RdBu;
else if(min >= 0) scl = container.sequential || scales.Reds;
else scl = container.sequentialminus || scales.Blues;

// reversescale is handled at the containerOut level
doUpdate('colorscale', scl, container.reversescale ? flipScale(scl) : scl);
Expand Down
9 changes: 7 additions & 2 deletions src/components/colorscale/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ var colorbarDefaults = require('../colorbar/defaults');
var isValidScale = require('./is_valid_scale');
var flipScale = require('./flip_scale');


module.exports = function colorScaleDefaults(traceIn, traceOut, layout, coerce, opts) {
var prefix = opts.prefix,
cLetter = opts.cLetter,
Expand All @@ -43,7 +42,13 @@ module.exports = function colorScaleDefaults(traceIn, traceOut, layout, coerce,
var autoColorscaleDflt;
if(sclIn !== undefined) autoColorscaleDflt = !isValidScale(sclIn);
coerce(prefix + 'autocolorscale', autoColorscaleDflt);
var sclOut = coerce(prefix + 'colorscale');

var layoutColorscale = layout.colorscale || {};
containerOut.diverging = layoutColorscale.diverging;
containerOut.sequential = layoutColorscale.sequential;
containerOut.sequentialminus = layoutColorscale.sequentialminus;
var dfltScl = containerOut.diverging;
var sclOut = coerce(prefix + 'colorscale', dfltScl);

// reversescale is handled at the containerOut level
var reverseScale = coerce(prefix + 'reversescale');
Expand Down
23 changes: 23 additions & 0 deletions src/plots/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,29 @@ module.exports = {
editType: 'calc',
description: 'Sets the default trace colors.'
},
colorscale: {
Copy link
Contributor

@etpinard etpinard Nov 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Let's put these new attributes and default logic in the Colorscale component module.

Create two new files:

  • src/components/colorscale/layout_attributes.js
  • src/components/colorscale/layout_defaults.js

and link their exports in src/components/colorscale/index.js as layoutAttributes and supplyLayoutDefaults. The latter will get automatically picked up in Plotly.supplyLayoutModuleDefaults here:

plotly.js/src/plots/plots.js

Lines 1512 to 1518 in 695f311

for(component in componentsRegistry) {
_module = componentsRegistry[component];
if(_module.supplyLayoutDefaults) {
_module.supplyLayoutDefaults(layoutIn, layoutOut, fullData);
}
}

sequential: {
valType: 'colorscale',
dflt: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

What would happen if we set dflt: scales.RdBu ?

Copy link
Contributor Author

@antoinerg antoinerg Nov 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

That's another way of doing it and I think it's much better than the current approach. We'd have the defaults clearly defined in the attributes where they should be 🎉

role: 'style',
editType: 'calc',
description: 'Sets the default sequential colorscale for positive values.'
},
sequentialminus: {
valType: 'colorscale',
dflt: false,
role: 'style',
editType: 'calc',
description: 'Sets the default sequential colorscale for negative values.'
},
diverging: {
valType: 'colorscale',
dflt: false,
role: 'style',
editType: 'calc',
description: 'Sets the default diverging colorscale.'
}
},
datarevision: {
valType: 'any',
role: 'info',
Expand Down
3 changes: 3 additions & 0 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,9 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) {
coerce('hidesources');

coerce('colorway');
coerce('colorscale.sequential');
coerce('colorscale.sequentialminus');
coerce('colorscale.diverging');

coerce('datarevision');

Expand Down
73 changes: 73 additions & 0 deletions test/jasmine/tests/colorscale_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,41 @@ describe('Test colorscale:', function() {
handleDefaults(traceIn, traceOut, layout, coerce, opts);
expect(traceOut.showscale).toBe(false);
});

it('should use global colorscale.diverging if no colorscale is specified', function() {
traceIn = {
zmin: -10,
zmax: 10
};
var divergingScale = [[0, 'rgb(0,0,0)'], [1, 'rgb(255,255,255)']];
var layout2 = {
colorscale: {
diverging: divergingScale
},
_dfltTitle: {colorbar: 'cb'}
};
handleDefaults(traceIn, traceOut, layout2, coerce, opts);
expect(traceOut.colorscale).toBe(divergingScale);
});

it('should not use layout colorscale.diverging if colorscale is specified', function() {
var divergingScale = [[0, 'rgb(0,0,0)'], [1, 'rgb(255,255,255)']];
var traceColorscale = [[0, 'rgb(128,128,128)'], [1, 'rgb(255,255,255)']];
traceIn = {
zmin: -10,
zmax: 10,
colorscale: traceColorscale
};

var layout2 = {
colorscale: {
diverging: divergingScale
},
_dfltTitle: {colorbar: 'cb'}
};
handleDefaults(traceIn, traceOut, layout2, coerce, opts);
expect(traceOut.colorscale).toBe(traceColorscale);
});
});

describe('handleDefaults (scatter-like version)', function() {
Expand Down Expand Up @@ -348,6 +383,41 @@ describe('Test colorscale:', function() {
handleDefaults(traceIn, traceOut, layout, coerce, opts);
expect(traceOut.marker.showscale).toBe(true);
});

it('should use layout colorscale.diverging if no colorscale is specified', function() {
traceIn = {
zmin: -10,
zmax: 10
};
var divergingScale = [[0, 'rgb(0,0,0)'], [1, 'rgb(255,255,255)']];
var layout2 = {
colorscale: {
diverging: divergingScale
},
_dfltTitle: {colorbar: 'cb'}
};
handleDefaults(traceIn, traceOut, layout2, coerce, opts);
expect(traceOut.marker.colorscale).toBe(divergingScale);
});

it('should not use layout colorscale.diverging if colorscale is specified', function() {
var divergingScale = [[0, 'rgb(0,0,0)'], [1, 'rgb(255,255,255)']];
var traceColorscale = [[0, 'rgb(128,128,128)'], [1, 'rgb(255,255,255)']];
traceIn = {
marker: {
colorscale: traceColorscale
}
};

var layout2 = {
colorscale: {
diverging: divergingScale
},
_dfltTitle: {colorbar: 'cb'}
};
handleDefaults(traceIn, traceOut, layout2, coerce, opts);
expect(traceOut.marker.colorscale).toBe(traceColorscale);
});
});

describe('calc', function() {
Expand All @@ -366,6 +436,7 @@ describe('Test colorscale:', function() {
autocolorscale: true,
_input: {autocolorscale: true}
};

z = [[0, -1.5], [-2, -10]];
calcColorscale(trace, z, '', 'z');
expect(trace.autocolorscale).toBe(true);
Expand All @@ -376,6 +447,8 @@ describe('Test colorscale:', function() {
trace = {
type: 'heatmap',
z: [[0, -1.5], [-2, -10]],
zmin: -10,
zmax: 0,
autocolorscale: true,
_input: {}
};
Expand Down