Skip to content

Config options coerce / schema #1188

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

Closed
wants to merge 6 commits into from
Closed

Config options coerce / schema #1188

wants to merge 6 commits into from

Conversation

etpinard
Copy link
Contributor

This PR builds on top of #1144 to bring config options into the plot schema.

image

This should be pretty useless in our API libraries - as config options will be listed as first classed attributes on-par with "data" / "layout".

This PR also make gd._context be filled using Lib.coerce making config option input more predictable internally.

- and rewrite setPlotConfig + setPlotContext
- so that dlft config gets coerced in gd._context
  (which is only done during Plotly.plot)
- which now require gd._context to be defined
  (that's for lib/queue.js)
@etpinard etpinard added this to the v1.21.0 milestone Nov 22, 2016
@etpinard
Copy link
Contributor Author

cc @cldougl @Kully it would be nice for plotly.py to use plot schema to fill in this list instead of the current hard-coded not-in-sync nightmare.

@etpinard
Copy link
Contributor Author

etpinard commented Nov 23, 2016

Moreover, (cc @cldougl @chriddyp @ellecj ) with this PR, the config options declarations now have the same format as the data and layout attributes. It might be nice to eventually display them on https://plot.ly/javascript/reference/ instead of the lonely, outdated https://plot.ly/javascript/configuration-options/

max: 4,
description: [
'Set the pixel ratio during WebGL image export.',
'This config option was formally named `plot3dPixelRatio`',
Copy link
Collaborator

Choose a reason for hiding this comment

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

formerly


// map plot3dPixelRatio to plotGlPixelRatio for backward compatibility
if(config.plot3dPixelRatio && !context.plotGlPixelRatio) {
context.plotGlPixelRatio = context.plot3dPixelRatio;
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this happen to the input object before coercing, so it can be validated? Anyway if it stays here it would need to be context.plotGlPixelRatio = config.plot3dPixelRatio;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment. I discovered some breaking behavior.

As this PR isn't important to anyone right now, I'll drop it from the 1.21.1 release.

@etpinard
Copy link
Contributor Author

Now that #1895 is merged, this PR is now very outdated, so I'll close it.

Go to #420 (comment) for a discussion on potential v2 config improvements.

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

Successfully merging this pull request may close these issues.

2 participants