MRG: use Matplotlib to detect new Matplotlib figures #1401
Conversation
Check Matplotlib fignums to look for new figures created during chunk.
|
I see that tests are failing, and detecting output differences, but I'm having some trouble replicating locally. Can I ask for hints as where to look to debug the test failures? |
|
Nice, this looks like a much better approach! Do you know if we have to cache the result of The relevant test file is: https://github.com/rstudio/reticulate/blob/main/tests/testthat/test-python-knitr-engine.R To run locally and interactively, from the package directory you can evaluate the R expressions: devtools::load_all()
testthat::test_file("tests/testthat/test-python-knitr-engine.R")The test snapshots and resources are available under the parent folder, named "testthat/resources" and "testthat/_snaps", respectively. Thank you! |
matthew-brett
force-pushed
the
mpl-figure-check
branch
from
5c145a9
to
0c1ce0e
Compare
|
OK - thanks - with those hints, I tracked down the failures. It turns out that, at least from document to document, there can be stray empty Matplotlib figures, resulting in multiple empty figures being generated, and then inserted into the output documents. For now, I've just checked if pending figures are empty, and closed those found to be empty. I did think about what to do if there were figures pending from before the chunk starts - but it seemed to me that a) I wasn't sure how those would come about, given the current mode, of always displaying figures if they arose in a chunk, and b) if they did come about - it seemed to me that the Jupyter way would be to display them - but I don't think there is a way to have panding figures from a Jupyter chunk. |
|
Am I right in thinking that the current test failures are unrelated? |
matthew-brett
force-pushed
the
mpl-figure-check
branch
from
0c1ce0e
to
9451cf8
Compare
|
While I was looking at it - is this line safe? https://github.com/rstudio/reticulate/blob/main/R/knitr-engine.R#L431 ? Won't it error if Matplotib is not installed, and the user is using RStudio desktop, for example? |
- reset matplot fig size each chunk closes rstudio#1398 - call plt.close() instead of plt.clf() to ensure that plt.get_fignums() is cleared and so we can easily avoid calling plt.show() too many times.
|
I made some commits to the branch, but they diverged with the most recent force-push. Re the |
|
Pushed another commit to https://github.com/rstudio/reticulate/tree/mpl-figure-check tweaking NEWS |
- reset matplot fig size each chunk closes rstudio#1398 - call plt.close() instead of plt.clf() to ensure that plt.get_fignums() is cleared and so we can easily avoid calling plt.show() too many times.
|
The failing tests are unrelated, they're due to a new scipy release. I'll fix on a different branch. |
|
Many thanks @matthew-brett! |
|
No problem, thanks for guiding it through….
…On Tue, 27 Jun 2023 at 15:16, Tomasz Kalinowski ***@***.***> wrote:
Many thanks @matthew-brett <https://github.com/matthew-brett>!
—
Reply to this email directly, view it on GitHub
<#1401 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAQQHCWTB5TVZGJC6CPZA3XNLTMFANCNFSM6AAAAAAZSMBP4I>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This uses what appears to be the standard
idiom
to detect pending Matplotlib plots.
With this PR, we do detect plots generated by code such as:
res = plt.plot(range(10))whereas previously we would have missed this as a plot generator, because
it was not a standalone expression.