DO NOT MERGE: Patch WebAgg backend for pyodide #29568
Conversation
| import tornado.web | ||
| import tornado.ioloop | ||
| import tornado.websocket | ||
| from js import document |
There was a problem hiding this comment.
We don't support matplotlib in Node.js anyways, but importing document in Node.js or other Node.js-like environments will fail.
| def open(self, fignum): | ||
| self.js_web_socket.open(create_proxy(self.on_message)) # should destroy proxy on close/exit? |
There was a problem hiding this comment.
Yes, I think it is better to destroy the proxy after use to prevent memory leaks. However, it should be ensured that the destroyed proxy is not accessed after its destruction.
There was a problem hiding this comment.
Indeed, so:
| self.js_web_socket.open(create_proxy(self.on_message)) # should destroy proxy on close/exit? | |
| self.on_message_proxy = create_proxy(self.on_message) | |
| self.js_web_socket.open(self.on_message_proxy) # should destroy proxy on close/exit? |
and then presumably in on_close we'd add self.on_message_proxy.destroy().
|
|
||
| WebAggApplication.start() | ||
| js_fig = run_js(js_code) |
|
|
||
| def on_close(self): | ||
| self.manager.remove_web_socket(self) | ||
|
|
||
| def on_message(self, message): | ||
| message = json.loads(message) | ||
| message = message.as_py_json() |
There was a problem hiding this comment.
Makes sense, assuming that we get a JavaScript JSON object (mix of arrays, objects, numbers, strings, and bools) as input.
| element.setAttribute( | ||
| "href", | ||
| "data:{};base64,{}".format( | ||
| mimetype, base64.b64encode(data.getvalue()).decode("ascii") | ||
| ), | ||
| ) | ||
| element.setAttribute("download", f"plot{figure_id}.{format}") | ||
| element.style.display = "none" | ||
| document.body.appendChild(element) | ||
| element.click() | ||
| document.body.removeChild(element) |
There was a problem hiding this comment.
I would have sworn there was a better way to do this by now. But maybe not?
There was a problem hiding this comment.
I copied this from matplotlib-pyodide. There must be a better way, I need to investigate.
There was a problem hiding this comment.
I haven't found a better way of doing this yet, so I am leaving it as it is for the moment.
| content = content.toJs(); | ||
| blob = new Blob([content]); |
There was a problem hiding this comment.
There's probably a way to do this with fewer copies.
There was a problem hiding this comment.
I found a better way using content.getBuffer() in 55117ee.
|
Thanks for the comments @ryanking13 and @hoodmane. I'm concluding that there isn't anything fundamentally wrong here and I will continue to work on it next week. |
This is the branch used to prepare
pyodidePR pyodide/pyodide#5398 to patch the Matplotlibwebaggbackend for use in pyodide. It is here in the form of a (never to be merged) PR so that it is easy to look at the differences in a split view. Note most of the changes are removals, followed by mock websockets on both the Python and JavaScript sides.