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.

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

Fix contour plot colorscale domain #6625

Merged
merged 15 commits into from
Jul 18, 2023
2 changes: 1 addition & 1 deletion src/components/colorbar/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ function drawColorBar(g, opts, gd) {
.data(fillLevels);
fills.enter().append('rect')
.classed(cn.cbfill, true)
.style('stroke', 'none');
.attr('style', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain this change?

Copy link
Contributor Author

@lvlte lvlte Jun 6, 2023

Choose a reason for hiding this comment

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

Well, it's a bit tricky to explain.. The first thing to note is that when calling Plotly.newPlot(), the colorbar is drawn twice, ie. the code above runs twice (which might be a bug (?) as the first draw seems correct, but I didn't dig much into that). When calling Plotly.react() though, it runs once.

Now, we have (before this commit) :

var fills = g.select('.' + cn.cbfills)
    .selectAll('rect.' + cn.cbfill)
    .attr('style', '')        // -> existing <rect> have empty `style` attribute
    .data(fillLevels);
fills.enter().append('rect')
    .classed(cn.cbfill, true)
    .style('stroke', 'none'); // -> missing <rect> are added with `style='stroke: none;'`
    fills.exit().remove();

So, if I'm not wrong :

  • After calling Plotly.newPlot(), because the code runs twice, there is no missing <rect> on the second pass obviously, and in the end all of them have an empty style attribute (for contour colorbar, each element get colored via the fill attribute)
  • After calling Plotly.react() however, because the code runs once, those <rect> that were newly added, if any, will have in the end style='stroke: none;'

This discrepancy causes this test case to fail, because some attributes are changing between transition/redraw. So this commit is just to fix that.

Now the question is : how the heck this test case hasn't ever failed until this PR ?

I think this could have to do with the double call and possible race conditions.. Hopefully I've been able to reproduce the failure without the colorbar fix, by using a mock with more data than those used previously by the test (longer to draw, hence the suspicion about race conditions), also fixed the promise chaining. I'm going to commit that so you can double check all of this.

fills.exit().remove();

var zBounds = zrange
Expand Down
18 changes: 15 additions & 3 deletions src/traces/contour/make_color_map.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ module.exports = function makeColorMap(trace) {

var si, i;

if(contours.coloring === 'heatmap') {
var zmin0 = cOpts.min;
var zmax0 = cOpts.max;
var zmin0 = cOpts.min;
var zmax0 = cOpts.max;
archmoj marked this conversation as resolved.
Show resolved Hide resolved

if(contours.coloring === 'heatmap') {
for(i = 0; i < len; i++) {
si = scl[i];
domain[i] = si[0] * (zmax0 - zmin0) + zmin0;
Expand Down Expand Up @@ -65,6 +65,18 @@ module.exports = function makeColorMap(trace) {
domain[i] = (si[0] * (nc + extra - 1) - (extra / 2)) * cs + start;
range[i] = si[1];
}

// Ensure zmin and zmax are included in the colorscale

if(domain[0] > zmin0) {
domain.unshift(zmin0);
range.unshift(range[0]);
}

if(domain[domain.length - 1] < zmax0) {
domain.push(zmax0);
range.push(range[range.length - 1]);
}
}

return Colorscale.makeColorScaleFunc(
Expand Down
Binary file modified test/image/baselines/25.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/axes_breaks-contour1d.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/axes_breaks-contour2d.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/cheater_contour.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/colorbar_tick_prefix_suffix.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/colorbar_tickformat.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/colorscale_opacity.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_edge_cases.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_label-formatting-via-colorbar.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_lines_coloring.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_nolines.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_nonlinear_interpolation.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_transposed.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/contour_xyz-gaps-on-sides.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/h-colorbar_contour_xyz-gaps-on-sides.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/h-colorbar_tickformat.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/h-colorbar_tickformat_with-border.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/heatmap_multicategory.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/histogram2dcontour_bingroup-coloraxis.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/multicategory_histograms.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/shared_coloraxes_contour.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion test/image/mocks/airfoil.json
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@
"zmax":1,
"name":"Pressure",
"colorscale":"Viridis",
"zmin":-8,
"zmin":-1,
"colorbar":{
"y":0,
"yanchor":"bottom",
Expand Down
10 changes: 10 additions & 0 deletions test/image/mocks/contour_edge_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
[0, 0, 0, -9, 0, 0],
[0, 0, 0, 0, 0, 0]],
"type": "contour",
"zmin": -0.000001,
"zmax": 0.000001,
"contours": {
"start": -0.000001,
"end": 0.000001,
Expand All @@ -55,6 +57,8 @@
[0, 0, 9, -9, 0, 0],
[0, 0, 0, 0, 0, 0]],
"type": "contour",
"zmin": -0.000001,
"zmax": 0.000001,
"contours": {
"start": -0.000001,
"end": 0.000001,
Expand Down Expand Up @@ -91,6 +95,8 @@
[0, 0, 0, 9, 0, 0],
[0, 0, 0, 0, 0, 0]],
"type": "contour",
"zmin": -0.000001,
"zmax": 0.000001,
"contours": {
"start": -0.0000005,
"end": 0.000001,
Expand All @@ -107,6 +113,8 @@
[0, 0, 0, 0, -9, 0],
[0, 0, 0, 0, 0, 0]],
"type": "contour",
"zmin": -0.000001,
"zmax": 0.000001,
"contours": {
"start": -0.0000005,
"end": 0.000001,
Expand All @@ -122,6 +130,8 @@
[0, 0, 0, 9, 0, 0],
[0, 0, 0, 0, 0, 0]],
"type": "contour",
"zmin": -0.000001,
"zmax": 0.000001,
"contours": {
"start": -0.0000005,
"end": 0.000001,
Expand Down
2 changes: 2 additions & 0 deletions test/image/mocks/contour_log.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
[6, 5, 4, 8, 2, 1],
[5, 4, 3, 2, 1, 0]
],
"zmin": 4,
"zmax": 6.5,
"contours": {"start": 4, "end": 6.5, "size": 1},
"type": "contour"
},
Expand Down
4 changes: 2 additions & 2 deletions test/image/mocks/contour_valid_ses.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
"end": 10,
"size": 1
},
"zmin": 0,
"zmax": 20
"zmin": 2,
"zmax": 10
}
],
"layout": {
Expand Down
2 changes: 1 addition & 1 deletion test/image/mocks/h-colorbar_airfoil.json
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@
"zmax":1,
"name":"Pressure",
"colorscale":"Viridis",
"zmin":-8,
"zmin":-1,
"colorbar":{
"orientation": "h",
"x":0.75,
Expand Down
2 changes: 2 additions & 0 deletions test/image/mocks/histogram2dcontour_bingroup-coloraxis.json
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,8 @@
}
],
"coloraxis": {
"cmin": 0,
"cmax": 8.5,
"colorscale": [
[
0,
Expand Down
4 changes: 4 additions & 0 deletions test/image/mocks/shared_coloraxes_contour.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@
}
},
"coloraxis2": {
"cmin": -5.5,
"cmax": 4.5,
"colorbar": {
"dtick": 1,
"len": 0.5,
Expand All @@ -120,6 +122,8 @@
}
},
"coloraxis3": {
"cmin": -5.5,
"cmax": 4.5,
"colorbar": {
"len": 0.5,
"x": -0.05, "xanchor": "right",
Expand Down