-
Notifications
You must be signed in to change notification settings - Fork 839
Unify handling of widget state/buffers/bufferPaths #7441
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull request overview
This pull request unifies the handling of widget state, buffers, and buffer paths in the anywidget integration, introducing a standardized "wire format" ({state, bufferPaths, buffers}) for transmitting data between Python backend and TypeScript frontend. The changes improve type safety, fix binary data transmission errors, and add comprehensive test coverage for the new encode/decode functions.
Key Changes
- Introduced wire format with explicit
encode_to_wire()anddecode_from_wire()functions in both Python and TypeScript to standardize binary buffer handling - Updated
anywidgetclass to use wire format internally while exposing decoded state to users via thevalueproperty - Added extensive test coverage for wire format encoding/decoding, including round-trip tests and edge cases
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
marimo/_plugins/ui/_impl/from_anywidget.py |
Added wire format types and encode/decode functions; updated anywidget class to use wire format internally |
marimo/_plugins/ui/_impl/anywidget/init.py |
Registered message handler for proper buffer handling |
marimo/_runtime/runtime.py |
Added logging for function call exceptions |
tests/_plugins/ui/_impl/test_anywidget.py |
Updated tests for wire format and added comprehensive TestWireFormat test class |
marimo/_smoke_tests/anywidget_examples/scatter.py |
Added new smoke test for jupyter-scatter widget |
frontend/src/utils/data-views.ts |
Implemented wire format serialization/deserialization with DataView handling |
frontend/src/utils/__tests__/data-views.test.ts |
Added comprehensive tests for wire format functions including immutability tests |
frontend/src/utils/json/base64.ts |
Added binaryToByteString utility function |
frontend/src/plugins/impl/anywidget/model.ts |
Updated Model class to handle wire format; changed dirty fields tracking to use Map |
frontend/src/plugins/impl/anywidget/__tests__/model.test.ts |
Updated tests to match new wire format and dirty fields behavior |
frontend/src/plugins/impl/anywidget/AnyWidgetPlugin.tsx |
Updated plugin to decode wire format and serialize changes back to wire format |
frontend/src/plugins/impl/anywidget/__tests__/AnyWidgetPlugin.test.tsx |
Updated tests to match new wire format |
Comments suppressed due to low confidence (1)
marimo/_runtime/runtime.py:1995
- This assignment to 'error_title' is unnecessary as it is redefined before this value is used.
This assignment to 'error_title' is unnecessary as it is redefined before this value is used.
This assignment to 'error_title' is unnecessary as it is redefined before this value is used.
This assignment to 'error_title' is unnecessary as it is redefined before this value is used.
error_title, error_message = "", ""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This keeps the
state/buffer/bufferPathsin sync when sending to/from the backend for the widget. This changes some typings and adds a few more tests. This is an improvement, but not the final state as we can hopefully simplify further. This does fix a few errors when sending back binary data.Prior work done by @manzt in 9426b13