Skip to content

Add missing fields to fullData - #2850

Merged
etpinard merged 8 commits into
masterfrom
missing-fulldata
Sep 12, 2018
Merged

Add missing fields to fullData#2850
etpinard merged 8 commits into
masterfrom
missing-fulldata

Conversation

@etpinard

@etpinardetpinard commented Jul 26, 2018

Copy link
Copy Markdown
Contributor

covers a few items in #2834


Does not yet address pie marker.colors and violin span and bandwidth (see #2834 (comment) for more on that topic).

cc @alexcjohnson

... instead of clearing them entirely, this is more
consistent with what the react-chart-editor expects
- this piece here is called by Colorbar.supplyDefaults, but
colorbars do not have a container-wide 'color' attribute unlike
axes, so the comparison with layoutAttributes.color was off.
- this bug did not affect the baselines as this routine is called
again during Colorbar.draw with a set container color.
Comment threadsrc/traces/box/plot.js Outdated
var paths = sel.selectAll('path.mean').data((
(trace.type === 'box' && trace.boxmean) ||
(trace.type === 'violin' && trace.box && trace.meanline)
(trace.type === 'violin' && (trace.box || {}).visible && (trace.meanline || {}).visible)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can't this just be trace.box.visible && trace.meanline.visible given the changes below?

... this reduces the occurance of intermittent failure in
the gl2d_click suite on etpinard's laptop
@etpinard

Copy link
Copy Markdown
ContributorAuthor

Editor devs, is adding violins to the chart editor a high priority?

If so, I'm thinking of simply mutating the computed span and bandwidth default values back into gd._fullDataandgd.data similar to how we currently handle histogram bins and contour levels.

@etpinard

Copy link
Copy Markdown
ContributorAuthor

Ooops, #2850 (comment) was meant for #2834

@etpinard

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson this PR isn't enough to cover all the items in #2834, but it does fix a few things. It would be nice to release this in next week's v1.41.0.

Tagging as reviewable.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yep, this is great, no need to wait on the stragglers before merging this. 💃

@etpinard
etpinard merged commit 40f6ce3 into masterSep 12, 2018
@etpinard
etpinard deleted the missing-fulldata branch September 12, 2018 14:37
@dmt0dmt0 mentioned this pull request Sep 12, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugsomething broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@etpinard@alexcjohnson