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

Don't mutate contours start/end/size into user traces #4199

Closed
mafar opened this issue Sep 18, 2019 · 4 comments
Closed

Don't mutate contours start/end/size into user traces #4199

mafar opened this issue Sep 18, 2019 · 4 comments
Labels
bug something broken

Comments

@mafar
Copy link
Contributor

mafar commented Sep 18, 2019

Demo: https://codepen.io/mafar/pen/NWKOxzr?editors=0010
This pen is bare minimum for bug reproduction

Explaination: https://plot.ly/javascript/reference/#contour
Consider a user is given 3 inputs where he has to set

  1. contours-start
  2. contours-end
  3. contours-size

and he types invalid option and plotly breaks

Required:
Instead of breaking, plotly must ignore values or throw warning

Image: wait to load
image

@mafar
Copy link
Contributor Author

mafar commented Sep 18, 2019

If plotly is initiated with wrong selection , it ignores values and does not break

      var data = [{
        z: [[10, 10.625, 12.5, 15.625, 20]],
        type: "contour",       
        contours: {
          start: 1000,
          end: 1
        }
      }];

But when same option is called by Plotly.restyle, plotly breaks

var update = {
  'contours.start': 1000,
  'contours.end': 1,
};
Plotly.restyle("myDiv", update, 0);

@mafar mafar changed the title [BUG][contour]Invalid options break library [BUG] data[type=contour].contours : Invalid options break library Sep 18, 2019
@etpinard
Copy link
Contributor

etpinard commented Sep 18, 2019

Thanks very much for reporting. This issue and #4201 have essentially the same cause: we mutate the input contours object during the contour calc step - see trace._input.contours assignments below:

// copy auto-contour info back to the source data.
// previously we copied the whole contours object back, but that had
// other info (coloring, showlines) that should be left to supplyDefaults
if(!trace._input.contours) trace._input.contours = {};
Lib.extendFlat(trace._input.contours, {
start: contours.start,
end: contours.end,
size: contours.size
});
trace._input.autocontour = true;

and

if(start > end) {
contours.start = inputContours.start = end;
end = contours.end = inputContours.end = start;
start = contours.start;
}
if(!(contours.size > 0)) {
var sizeOut;
if(start === end) sizeOut = 1;
else sizeOut = autoContours(start, end, trace.ncontours).dtick;
inputContours.size = contours.size = sizeOut;
}


In brief,

Plotly.newPlot(gd,  [{
    z: [
      [10, 10.625, 12.5, 15.625, 20],
      [5.625, 6.25, 8.125, 11.25, 15.625],
      [2.5, 3.125, 5, 8.125, 12.5],
      [0.625, 1.25, 3.125, 6.25, 10.625],
      [0, 0.625, 2.5, 5.625, 10]
    ],
    type: "contour",
    colorscale: "Jet",
    ncontours: 15,
    autocontour: false 
  }])

// gives
gd.data[0].contours // => {start: 2, end: 18, size: 2}
// ... as we copied in the "auto" contour computed values

// then
Plotly.restyle(gd, {'contours.start': 1000})
// gives
gd.data[0].contours // => {start: 1000, end: 18, size: 2}

To solve this problem the right way, we'll need to implement "relink" logic similar to what #3341 did for colorscale attributes and what #3044 did for histogram bins.

Note that in the meantime, using

Plotly.restyle(gd, {contours: {start: 1000}})
// which replaces all of `gd.data[0].contours` with `{start: 1000}` and gives
gd.data[0].contours // => {start: 2, end: 18, size: 2}
// ... as start > end is not accepted

might be a sufficient workaround.

@etpinard
Copy link
Contributor

Moreover, we should probably make attributes contours.start, contours.end and contours.size

start: {
valType: 'number',
dflt: null,
role: 'style',
editType: 'plot',
impliedEdits: {'^autocontour': false},
description: [
'Sets the starting contour level value.',
'Must be less than `contours.end`'
].join(' ')
},
end: {
valType: 'number',
dflt: null,
role: 'style',
editType: 'plot',
impliedEdits: {'^autocontour': false},
description: [
'Sets the end contour level value.',
'Must be more than `contours.start`'
].join(' ')
},
size: {
valType: 'number',
dflt: null,
min: 0,
role: 'style',
editType: 'plot',
impliedEdits: {'^autocontour': false},
description: [
'Sets the step between each contour level.',
'Must be positive.'
].join(' ')
},

editType: 'calc' (not 'plot').

@etpinard etpinard added the bug something broken label Sep 18, 2019
@etpinard etpinard changed the title [BUG] data[type=contour].contours : Invalid options break library Don't mutate contours start/end/size into user traces Sep 18, 2019
@gvwilson
Copy link
Contributor

Hi - we are currently trying to tidy up Plotly's public repositories to help us focus our efforts on things that will help users most. Since this issue has been sitting for several years, I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our backlog. Thanks for your help - @gvwilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

3 participants