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

Check to make sure children are not nested lists. Fix plotly/dash-renderer#23 #330

Closed
wants to merge 1 commit into from

Conversation

rmarren1
Copy link
Contributor

Proposed fix to plotly/dash-renderer#23

This solution borrows heavily from #273. Basically the same except instead of checking if values in the tree are a member of a valid set, we just check if the value is not a nested list.

A major difference is that this check if performed before each callback rather than being called only if json.dumps fails. In this case, json.dumps succeeds since nested arrays are valid JSON.

Examples:

dash.exceptions.InvalidCallbackReturnValue: 
The callback for property `children` of component `output`
returned a tree with one value having type `list`
which is not JSON serializable.

The value in question is located at
[*] list 


and has string representation
`['nested']`

In general, Dash properties can only be
dash components, strings, dictionaries, numbers, None,
or un-nested lists of those.

dash.exceptions.InvalidCallbackReturnValue: 
The callback for property `children` of component `output`
returned a tree with one value having type `list`
which can not be serialized by Dash.

The value in question is located at
[*] Div 


and has string representation
`[['nested']]`

In general, Dash properties can only be
dash components, strings, dictionaries, numbers, None,
or un-nested lists of those.

@chriddyp
Copy link
Member

A major difference is that this check if performed before each callback rather than being called only if json.dumps fails. In this case, json.dumps succeeds since nested arrays are valid JSON.

This is an interesting approach. I'm a little worried about:

  • Performance issues in crawling the tree, especially with really large apps
  • Keeping this in check with our json serializer. I believe that children=pandas.Series(...) is technically valid but it would fail in this case.

Architecturally, I originally imagined this working on the component-level. That way, we could verify:

  • All properties, not just the children prop
  • Not just primative types but also shapes and nested types

In the most general solution, I imagined this as being part of the base Component and doing type-checking with the metadata that we are provided through propTypes. That way, the component author could specify the shapes of their props and the validation would automatically be done. I have some more details about this here: #264

If we ended up implementing #264, then I think that we would get this issue (children as nested children) for free. Of course, there are still some issues:

  • The higher-level types. Technically a pandas.Series should be considered valid just like a list. Perhaps for each propType we implement a list of valid Python types or validators
{
    'array': [pandas.Series, list, numpy.array]
}
  • We might want to make it configurable (similar to suppress_callback_exceptions, something like suppress_component_validation_exceptions), in case the component author didn't fully annotate their proptypes

@rmarren1
Copy link
Contributor Author

closed in favor of more general #340

@rmarren1 rmarren1 closed this Aug 17, 2018
HammadTheOne pushed a commit to HammadTheOne/dash that referenced this pull request May 28, 2021
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022
Load CSS in webpack with style-loader and css-loader
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

Successfully merging this pull request may close these issues.

2 participants