-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Conversation
- 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)
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`', |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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;
There was a problem hiding this comment.
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.
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. |
This PR builds on top of #1144 to bring
config
options into the plot schema.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 usingLib.coerce
making config option input more predictable internally.