-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
fix plt.show doesn't warn if a non-GUI backend #10685
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
We already have runtime tests for LaTeX and associated dependencies; the check in setup.py is not used for anything.
Previously, changes to the text.latex.unicode or text.latex.preview rcParams (which do change the autogenerated preamble) would not invalidate the tex cache.
1) math.ceil() returns an int on Py3. 2) Window.SetInitialSize and Window.IsShownOnScreen are documented in the wx API; it seems overkill to try to workaround their possible absence (that code was present from the very beginning of the git history). 3) the macros attribute is unused since 7499f7c (2004).
Fix some doc links Put back AGG OO example Remove old example
Thanks for the PR! I just editied the title to be more descriptive and to exclude the issue number (though definitely include the issue # in the PR summary!) |
lib/matplotlib/backend_bases.py
Outdated
import warnings | ||
warnings.warn( | ||
"matplotlib is currently using a non-GUI backend, " | ||
"so cannot show the figure") |
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.
Might be nice to say which backend is being used
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.
Also, what's the policy now for warnings versus logging warnings?
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.
According to the logging howto
warnings.warn() in library code if the issue is avoidable and the client application should be modified to eliminate the warning
logging.warning() if there is nothing the client application can do about the situation, but the event should still be noted
The former is the case here. One should not try to call show (or expect that it does something) on a non-GUI backend. So, warnings.warn is correct.
lib/matplotlib/backend_bases.py
Outdated
@@ -175,20 +175,28 @@ def draw_if_interactive(cls): | |||
cls.trigger_manager_draw(manager) | |||
|
|||
@classmethod | |||
def show(cls, block=None): | |||
def show(cls, block=None, warn=True): |
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.
I don't think we need the warn
parameter. Client applications can use the filter mechanism of the warnings module, if they really want to suppress the warning.
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.
ok, I'll remove this parameter. Thanks.
db846a1
to
63f468e
Compare
Hi @jklymak , I have removed the warning parameter and added a test. Is there anything else I should do to make this better? |
|
||
def test_NonGui_warning(): | ||
matplotlib.use('Agg') | ||
import matplotlib.pyplot as plt |
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.
Duplicate import.
matplotlib.use('Agg') | ||
import matplotlib.pyplot as plt | ||
plt.plot([1, 2, 3]) | ||
plt.savefig('myfig') |
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.
Not needed?
@@ -179,3 +179,22 @@ def psfont(*args, **kwargs): | |||
ax.text(0.5, 0.5, 'hello') | |||
with tempfile.TemporaryFile() as tmpfile, pytest.raises(ValueError): | |||
fig.savefig(tmpfile, format='svg') | |||
|
|||
|
|||
def test_NonGui_warning(): |
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.
I'm a bit confused; why does this test belong in the SVG backend test file? (The name should also not be CamelCase.)
lib/matplotlib/backend_bases.py
Outdated
import warnings | ||
warnings.warn( | ||
"matplotlib is currently using a non-GUI backend, " | ||
"so cannot show the figure") |
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.
It looks like you missed @WeatherGod's suggestion:
Might be nice to say which backend is being used
|
||
|
||
def test_NonGui_warning(): | ||
matplotlib.use('Agg') |
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.
To force an alternate backend, you can mark the function with a decorator: @pytest.mark.backend('Agg')
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.
modulo the docstring typo
warnings.warn( | ||
"matplotlib is currently using a non-GUI backend, " | ||
"so cannot show the figure") | ||
('matplotlib is currently using %s, which is a ' + |
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.
Can you fix the docstring typo while you are working on this?
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.
sorry, I don't find the typo. Which line is it?
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.
"warm" should be "warn"... Unless my screen is blurry.
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... I think so. It is warn. :)
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.
Line 434/435
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.
oh I see it. Will fix. Thank you
The workaround for strftime with dates < 1900 is not needed anymore since Py 3.3, see note (2) in https://docs.python.org/3/library/datetime.html#strftime-strptime-behavior
Invalidate texmanager cache when any text.latex.* rc changes.
Rely a bit more on rc_context.
Move some examples out of examples/api
Remove the 'hold' kwarg from codebase
The int version of the buffer size was not updated when the buffer was resized. It's there to prevent a signed/unsigned comparison warning, but it's simpler just to cast the other side of the comparison. There's no problem with the signed-to-unsigned cast since we already know that the result is positive due to the previous check. Fixes matplotlib#10889.
FIX: axes limits reverting to automatic when sharing
Fix overflow when resizing path-to-string buffer.
Note that plt.spectral already raises an exception as of 2.2 anyways (the underlying colormap has already been removed).
Remove deprecated code in image.py
API: Do not suggest a unique filename in save dialogue
DOC: Update definition of area in scatter examples
…b#10914) API: set aspect in Axes.pie to be 'equal' * Changed pie charts default shape to circle (instead of oval), and added tests * Updated comments for default circular pie chart * fix pep8 * removed unneccessary test case and updated docstring * added what's new entry and edited docstring for pie * fix pep8 * fixed typo
Use function signatures in boilerplate.py.
Some more removals of deprecated APIs.
Replace "matplotlibrc" by "rcParams" in the docs where applicable.
FIX: ioerror font cache, second try
…arkers FIX: Postscript allow empty markers
Declare global variables that are created elsewhere
…ao/matplotlib into CinnyCao-becky-fix-matplotlib#7908 Fix the merge conflict
I merged master into this branch to address the merge conflict, and now Github is showing this as a really huge patch (+3,415 −5,454). But on the command line this still shows up as the same change as before. I wonder if Github is confused or if I am mistaken... |
Rebasing on to master should fix things I think. |
fix plt.show doesn't warn if a non-GUI backend
I think the Github diff machinery just has some limits for how complicated commit graphs it can handle. There were some old PRs (at least one from October 2017) merged recently, so if you look at @zhoubecky Thank you for your contribution! |
PR Summary
fix plt.show doesn't warn if a non-GUI backend is being used #7908
will add command and unit tests later