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

Receiving binary buffers from frontend fails #1592

Closed
vidartf opened this issue Aug 5, 2017 · 4 comments · Fixed by #1595
Closed

Receiving binary buffers from frontend fails #1592

vidartf opened this issue Aug 5, 2017 · 4 comments · Fixed by #1595
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@vidartf
Copy link
Member

vidartf commented Aug 5, 2017

If receiving buffers from the front end, the following line fails: https://github.com/jupyter-widgets/ipywidgets/blob/master/ipywidgets/widgets/widget.py#L601

The error is TypeError: <memory at 0x000001E2CA4F8708> is not JSON serializable.

An outline of the trace is:

  • Comm message with buffer received. Calls set_state.
  • set_state:
    • Locks the properties.
    • Deserializes the data (to numpy array in this case).
    • Calls set_trait.
  • notify_change calls _should_send_property.
  • to_json (my serializer) gives back a dict with a memoryview.
  • jsondumps chokes on the memoryview.
@vidartf vidartf changed the title Sending binary buffers from frontend fails Receiving binary buffers from frontend fails Aug 5, 2017
@vidartf
Copy link
Member Author

vidartf commented Aug 5, 2017

A possible solution is to do the comparison in the the serialized state after calls to _separate_buffers. The buffers can be compared by instance (a is b) or possibly by numpy (that should maybe be left as an extension point for custom comparison).

@jasongrout
Copy link
Member

jasongrout commented Aug 5, 2017

Ah yes, good catch. That is from #1523. I like your solution.

We've tried to have a lot better testing around the syncing framework on the javascript side. We should have a unit test for this on the python side too - that would have caught this when the PR was merged.

@jasongrout jasongrout added this to the 7.0 milestone Aug 5, 2017
@vidartf
Copy link
Member Author

vidartf commented Aug 5, 2017

I'll PR a fix later. Gonna try to add a test as well!

@jasongrout
Copy link
Member

Awesome, thanks!

@github-actions github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 11, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants