Skip to content

Expose info about attribute coercion and/or way to validate user data/layout #598

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
etpinard opened this issue Jun 2, 2016 · 6 comments · Fixed by #697
Closed

Expose info about attribute coercion and/or way to validate user data/layout #598

etpinard opened this issue Jun 2, 2016 · 6 comments · Fixed by #697
Assignees
Labels
feature something new

Comments

@etpinard
Copy link
Contributor

etpinard commented Jun 2, 2016

Two ways of doing this come to mind:

  1. Add a few Lib.log in coerce.js notifying users if a given attribute is set to an invalid value. Post Log levels #590, these logs would be hidden by default. This method has one drawback: it would only warn user about invalid values, not about invalid attributes (which never reach coerce.js.
  2. Add a Plotly.validate(data, layout) method that runs the full supplyDefaults chain, compare input and full data/layout and logs the info.
@cpsievert
Copy link

As mentioned in #619, having a required meta tag for attributes would be quite useful for generating informative error messages in our client APIs

@etpinard
Copy link
Contributor Author

I just pushed some Proof-of-Concept commits for a Plotly.validate method to plotly-validate.

I wouldn't mind getting opinions from @cpsievert @mdtusz @alexcjohnson @chriddyp @cldougl

@etpinard
Copy link
Contributor Author

Note that the idea of providing an error code comes from the deep-diff package.

I'm imagining that for some applications e.g. the unsettable validate error code would not matter.

@etpinard etpinard added the feature something new label Jun 21, 2016
@alexcjohnson
Copy link
Collaborator

@etpinard I love the strategy and the structure of the response. Aside from a few comments I made on the commits, I would just try and make the error codes look more like errors than benign "differences" like those returned by deep-diff. So instead of "visible", use "invisible" or even "aborted" because that's likely what happened - coercion was aborted because something crucial was missing. "unsettable" I might just call "unused", and it needs more explanation like "key XXX is valid but was not used because other values made it irrelevant"

@mdtusz
Copy link
Contributor

mdtusz commented Jun 21, 2016

Looks pretty good. Github won't let me comment on the code for some reason, but it may be nice to pass a list of the path keys rather than just root to crawl. E.g. ['xaxis', 'rangeslider', 'range'] so that if there arises any specific validation based on the nest, we can test it.

@alexcjohnson
Copy link
Collaborator

You can't comment on the combined diff until it's in a PR I guess... But you can comment on individual commits

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 a pull request may close this issue.

4 participants