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

Violins and Boxes not always honoring 'group' #3390

Closed
nicolaskruchten opened this issue Jan 3, 2019 · 8 comments
Closed

Violins and Boxes not always honoring 'group' #3390

nicolaskruchten opened this issue Jan 3, 2019 · 8 comments
Assignees

Comments

@nicolaskruchten
Copy link
Member

I'm seeing some odd inconsistencies in overlay vs group modes with boxes/violins... In all of the following cases, violinmode = "group":

image

image

image

image

The last one is where the problem is: why are they overlaid?

@etpinard
Copy link
Contributor

etpinard commented Jan 3, 2019

The last one is where the problem is: why are they overlaid?

Would it be too much to ask to share the "data" / "layout" of that graph. I'm not too familiar with px (yet). Thanks!

@nicolaskruchten
Copy link
Member Author

@nicolaskruchten
Copy link
Member Author

nicolaskruchten commented Jan 4, 2019

OK, so the reason it's not grouping is because of this block:

// if there's no duplication of x points,
// disable 'group' mode by setting counter to 1
if(pointList.length === boxdv.vals.length) {
fullLayout[numKey] = 1;
}

I'm not super clear on what it's doing or why but temporarily deleting that block does force things back into grouping mode, like it says in the comment. BUT I then get this odd behaviour, where it's grouping "too much"... it's like it doesn't think that the various "female" groups are the same and so it's leaving a bunch of extra room in each subplot :) ... split off into #3402

image

@nicolaskruchten
Copy link
Member Author

Maybe related? plotly/plotly.py#1389

@nicolaskruchten
Copy link
Member Author

nicolaskruchten commented Jan 5, 2019

Hmm actually the “too much grouping” is present in the first two screenshots as well, just less obviously-so. That might just be a separate issue. (see #3402)

@nicolaskruchten nicolaskruchten changed the title Violins and Boxes not honoring 'group' Violins and Boxes not always honoring 'group' Jan 5, 2019
@etpinard etpinard self-assigned this Jan 7, 2019
@etpinard
Copy link
Contributor

etpinard commented Jan 16, 2019

Consider the following:

Plotly.newPlot('graph', [{
  type: 'box',
  x0: 1,
  y: [1,2,1,2,1,2,3,4,4],
}, {
  type: 'box',
  x0: 2,
  y: [2,1,2,3,3,1,0,0,1]
}], {
  boxmode: 'group'
})

which currently looks like:

image

codepen: https://codepen.io/etpinard/pen/GPLXWX?editors=1010

Now after 🔪 ing block

// if there's no duplication of x points,
// disable 'group' mode by setting counter to 1
if(pointList.length === boxdv.vals.length) {
fullLayout[numKey] = 1;
}

like it was proposed in #3390 (comment), we get

image


In brief, currently we "disable" grouping when there are as many distinct box/violin positions as there are box/violin traces. One could argue that this piece of logic is too magical. Afterall, boxmode: 'overlay' by default: if we 🔪 boxmode in the above snippet, we get

image

both on master and after 🔪 ing the aforementioned block that "disables" grouping.


The case brought in https://plot.ly/~nicolaskruchten/252 is even more subtle. Here, there are as many distinct violin positions as there are violin traces on only one particular subplot (the bottom-left one, 1 violin position for 1 violin trace). So, this disables grouping for all subplots. This is obviously wrong: that check should happen at the graph-level, not the subplot-level. Fixing this would be easy.

That said, instead of just fixing the above case, should we instead remove the magical disable-grouping block altogether?

@nicolaskruchten
Copy link
Member Author

I would be fine with removing this magic, given that it can be turned on explicitly with overlay. This would cause further deviations from what bar does in group mode, however (which is basically to have the bars stretch to fill the available space, no matter how many there are).

@etpinard
Copy link
Contributor

done in #3445

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

No branches or pull requests

2 participants