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

Prune unsupported global-level trace attributes #3058

Closed
alexcjohnson opened this issue Sep 28, 2018 · 4 comments
Closed

Prune unsupported global-level trace attributes #3058

alexcjohnson opened this issue Sep 28, 2018 · 4 comments

Comments

@alexcjohnson
Copy link
Collaborator

There are a bunch of attributes that we insert in the schema, and largely include in _fullData for all trace types. But many trace types don't support all of these attributes. As a simple workaround in @jonmmease's parcats PR, 273d2d0 (following @etpinard's suggestion #2963 (comment)) hides a the ones parcats doesn't use from the schema, but they will still show up in _fullData.

As I said in #2963 (comment):

There's a module categories: ['noOpacity'] which prevents coercing opacity (so it's not in fullData, which is important for the editor) but does not take it out of the schema (and anyway only handles opacity). @etpinard 's solution would fix the schema but not fullData. Sounds like what we really want is something like:

module.omitGlobalAttrs: ['selectedpoints', 'hoverlabel', 'opacity', 'ids', 'customdata']

that would do both.

There are probably quite a few other traces that omit some of these - in fact, probably everything that sets traceOut._length = null has no use for selectedpoints, ids, and customdata...

And @etpinard responded #2963 (comment):

Good point here. But I think for this PR my suggestion in #2963 (comment) will be good enough. The react-chart-editor (aka RCE) doesn't know how to handle traces with dimensions yet, so having a few extra keys in fullData won't affect anyone at this stage. But yeah, once merged we should open a new issue (similar to #2834 and #2908) about this.

So here's that issue :)

@alexcjohnson
Copy link
Collaborator Author

There's a related issue with the way we merge base and module attributes in the schema: sometimes extendDeepAll goes too far, as in:

> Plotly.PlotSchema.get().traces.scatterpolar.attributes.hoverinfo.flags
<- ["r", "theta", "text", "name", "name"]

where the second "name" came from the base ["x", "y", "z", "text", "name"] getting added to a length-4 override in the module. We should really do a more manual crawl, only adding base attributes if they haven't been overridden in the module.

Anyway a lot of modules that have no "z" inappropriately inherit it as well, this should really perhaps always be handled at the module level.

@etpinard
Copy link
Contributor

After @antoinerg 's #3158

  • hoverinfo flags are now extended properly in trace attribute module
  • hoverlabel is now omitted in attributes and not coerced for traces that don't support it

Now, me and @alexcjohnson disagree on what to do with ids and customdata. We'll leave this issue open until we reach a consensus.

@antoinerg
Copy link
Contributor

After @antoinerg's #3158

  • transforms was removed from carpet, cone, contourcarpet, mesh3d, heatmap, parcoords, pointcloud, sankey, streamtube, surface and table

@etpinard
Copy link
Contributor

etpinard commented Apr 5, 2019

  • Traces w/o the showLegend category should not have showlegend, legendgroup in their schema

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

No branches or pull requests

3 participants