-
Notifications
You must be signed in to change notification settings - Fork 92
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
Implement a custom matplotlib backend for positron #2765
Conversation
On quick initial testing, this looks great, and answers a long standing question about plot incremental updates. Once you resolve the new conflicts, let's quickly chat - I think the need for show() by default is fine in the REPL... but want to chat about shared state between plt usages. |
0322f0b
to
6973964
Compare
I've fixed the merge conflicts. Happy to chat! |
Called by matplotlib when a figure is shown via `plt.show()` or `figure.show()`. | ||
""" | ||
if self._plot is None: | ||
self._plot = self._plots_service.create_plot(self.canvas._render, self._handle_close) |
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.
In this path, i.e. the first time, does self._plot.show() not get called?
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.
Yes, the explicit show isn't needed in that case because when the frontend receives the comm open message, it responds with a render request. I'll add a comment.
extensions/positron-python/python_files/positron/positron_ipykernel/matplotlib_backend.py
Show resolved
Hide resolved
…ernel/matplotlib_backend.py Signed-off-by: Wasim Lorgat <[email protected]>
This is really great, one thing I'd like to brainstorm on is whether there are any enhancements in a future PR we can make to communicate the "current" context of matplotlib, i.e. which figure, or potentially more (axes etc). is current. When working on separate figures, some of the API seems to require you to change the "current" figure to mutate it, and keeping track of what is current may be useful feedback in the UI? |
Intent
Address #2748.
Approach
This implements a custom matplotlib backend for Positron in the
matplotlib_backend
module. I believe this is the intended interface to connect matplotlib to a new frontend, and it allows us much tighter integration between matplotlib events and the Positron frontend.I refactored the plots service (
plots.py
) to be independent of the plotting backend framework (in this case matplotlib). In theory, the plots service should easily be able to connect to other Python plotting frameworks.I also added the
show
event to the plots comm to focus an existing plot without requesting a re-render.See the linked issue description and tests for more details.
QA Notes
See the linked issue and the contributed tests for QA test cases. Note that notebook plotting behavior should be unaffected by this PR.