-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Give a more informative error for JSON not serializable. (#269) #273
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
Changes from 5 commits
433a65e
6bba6e9
ba33c6e
ab441f4
7f609e8
ee8a489
ed4ecf0
718e259
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,3 +44,7 @@ class CantHaveMultipleOutputs(CallbackException): | |
|
||
class PreventUpdate(CallbackException): | ||
pass | ||
|
||
|
||
class ReturnValueNotJSONSerializable(CallbackException): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe just me, but I don't like that exception name, it gives too much info on the cause while not giving the true nature of the error, that is the return value of a callback is invalid. I would change it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to me, pushed those changes. |
||
pass |
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.
Since we JSON serialize with the
json.dumps(obj, cls=plotly.utils.PlotlyJSONEncoder)
, this list actually has a few other items: https://github.com/plotly/plotly.py/blob/6b3a0135977b92b3f7e0be2a1e8b418d995c70e3/plotly/utils.py#L137-L332So, if there was a component property that accepted a list of numbers, then technically the user could return a numpy array or a dataframe.
The only example that immediately comes to mind is updating the
figure
property in a callback with theplotly.graph_objs
. These objects get serialized because they have ato_plotly_json
method.So, I wonder if instead of validating up-front, we should only run this routine only if our
json.dumps(resp, plotly.utils.PlotlyJSONEncoder)
call fails.This would have the other advantage of being faster for the non-error case. If the user returns a huge object (e.g. some of my callbacks in apps return a 2MB nested component), doing this recursive validation might be slow.
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.
That makes sense. I am not sure how large of a speed up that would be since validating a large callback output is likely much faster than the network delay of sending that large output, but it definitely makes it easier than crafting a perfect validator (that would need to update every time we want to extend
PlotlyJSONEncoder
).