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

Remove erroneous traces in multi-subplot layout containing fill-to scatter traces #1198

Merged
merged 2 commits into from
Nov 28, 2016

Conversation

etpinard
Copy link
Contributor

fixes #1196

which were broken since 1.17.0 by during the animation push of PR #802

In brief, erroneous traces are currently appearing when a cartesian trace in one subplot is followed by a scatter trace with fill set to one of ['tonextx', 'tonexty', 'tonext] - like in plotly.py-manufactured violins as discovered in #1196

The fix is pretty straight forward: when adding previous traces to a trace module calcdata array (done to get fills right), we now make sure that the previous traces are on the same subplot as the current traces. This PR also adds a mock 🔒 ing down plotly.py violins.

@etpinard etpinard added status: reviewable bug something broken labels Nov 24, 2016
@@ -0,0 +1,3659 @@
{
"data": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plotting this mock with the current master gives:

image

cdSubplot.indexOf(pcd) === -1) {
if(
pcd &&
pcd[0].trace.xaxis + pcd[0].trace.yaxis === subplot &&
Copy link
Contributor Author

@etpinard etpinard Nov 24, 2016

Choose a reason for hiding this comment

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

@rreusser does this look right to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh, I see. cd is all the subplots. Yup, fix looks good to me.

@rreusser
Copy link
Contributor

💃

@etpinard etpinard added this to the v1.21.0 milestone Nov 28, 2016
@etpinard etpinard merged commit e91c3bc into master Nov 28, 2016
@etpinard etpinard deleted the fix-violins branch November 28, 2016 15:06
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

Successfully merging this pull request may close these issues.

broken violin plot
2 participants