Skip to content
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

Linestyle pattern depends on current style, not style set at creation #6592

Closed
dopplershift opened this issue Jun 16, 2016 · 12 comments
Closed

Comments

@dopplershift
Copy link
Contributor

dopplershift commented Jun 16, 2016

This is on 2.x (both the beta and current git HEAD) and confused me for the longest time:

from matplotlib import style
import matplotlib.pyplot as plt

def make_fig():
    fig = plt.figure()
    ax = fig.add_subplot(1, 1, 1)
    ax.plot([1, 2, 3], linestyle='dashed')
    ax.grid(True)
    return fig

with style.context('classic'):
    fig = make_fig()
    fig.savefig('test1.png')
fig.savefig('test2.png')

fig = make_fig()
fig.savefig('test3.png')

So the first save_fig() gives the classic style as expected:
test1
And the last save_fig() gives the new defaults:
test3
But for some reason the 2nd savefig(), where the figure is created with the classic style, but is written outside the context manager gives a figure that looks like classic, except the dash pattern different:
test2
This seriously confused me for the afternoon--it wreaks havoc with my tests where I'm now setting the stylesheet to classic inside the test. When pytest-mpl saves the test outside the context manager (and outside my test function) I get some weird abomination.

@dopplershift dopplershift added this to the 2.0 (style change major release) milestone Jun 16, 2016
@efiring
Copy link
Member

efiring commented Jun 16, 2016

When savefig is executed, it draws the figure. If this is outside the context manager, then all operations that are done at drawing time will be without that context. Anything that was established before drawing time will be with the context. I don't think there is any way to get around this--it's fundamental to the way mpl and context managers work. Some things are figured out a drawing time, others at object creation time. The particular case here where the dash pattern matches neither style is a surprise, I agree.

@WeatherGod
Copy link
Member

There are a bunch of oddities here. Notice that the font size seems
smaller, and it seems to me that the line width is slightly smaller, too? I
wonder if the dpi or the figure size is getting messed up, too? If so,
could this explain some of our intermittent test failures where the result
image has the wrong size?

On Thu, Jun 16, 2016 at 1:53 AM, Eric Firing notifications@github.com
wrote:

When savefig is executed, it draws the figure. If this is outside the
context manager, then all operations that are done at drawing time will be
without that context. Anything that was established before drawing time
will be with the context. I don't think there is any way to get around
this--it's fundamental to the way mpl and context managers work. Some
things are figured out a drawing time, others at object creation time. The
particular case here where the dash pattern matches neither style is a
surprise, I agree.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#6592 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AARy-N40XFNeyrtjWqVFWJU9lX_Msdbdks5qMOTcgaJpZM4I3BAz
.

@tacaswell
Copy link
Member

Any of the draw-time consultations of checking for the internal classic style need to turn into caching that on the artists 😞

@dopplershift
Copy link
Contributor Author

To say it's surprising is an understatement. At a bare minimum, this needs to be in bold print with the docs for style.context.

I recognize how much of a pain in the ass it would be to fix this completely--but I feel confident in saying that having these 3 possibilities is a terrible user experience (though hopefully not frequently encountered). If we agree that in principle that savefig() should produce the expect results when called outside the context manager, then at least we know what to do when issues like this crop up.

@efiring
Copy link
Member

efiring commented Jun 16, 2016

On 2016/06/16 5:20 AM, Ryan May wrote:

If we agree that in principle that |savefig()| should produce the expect
results when called outside the context manager, then at least we know
what to do when issues like this crop up.

If I understand you correctly, then I don't agree with this in
principle. From creating the Figure to the last savefig, figure
generation has to be either in a context, or outside. It doesn't make
sense to be half in and half out. That's the point of a context
manager. When you leave the context, you've finished with it and reset
whatever state setting was done. Maybe the problem is that the context
manager is not the appropriate tool here; or it should be a Figure
generation context, ensuring the Figure is closed upon exit.

@dopplershift
Copy link
Contributor Author

Well, I would have expected the following to work

from matplotlib import style
import matplotlib.pyplot as plt

fig = plt.figure()
ax = fig.add_subplot(1, 1, 1)

with style.context('classic'):
    ax.plot([1, 2, 3], linestyle='dashed')

with style.context('default'):
    ax.plot([2, 4, 6], linestyle='dashed')

fig.savefig('mixed.png')

But I now see the semantics of it (especially with property cycles) are a bit complicated. So in lieu of that, is there any mechanism that gives something like:

fig = plt.figure()
fig.stylesheet = 'classic'

What I'm running up against is the problem that styles are a global state, and I really don't want to be mucking with global state in tests.

@dopplershift
Copy link
Contributor Author

dopplershift commented Jun 16, 2016

Oh, and I just found this in the stylesheet docs:

>>> import numpy as np
>>> import matplotlib.pyplot as plt
>>>
>>> with plt.style.context(('dark_background')):
>>>     plt.plot(np.sin(np.linspace(0, 2 * np.pi)), 'r-o')
>>>
>>> # Some plotting code with the default style
>>>
>>> plt.show()

So whoever wrote this intended it to work as I expected....

@efiring
Copy link
Member

efiring commented Jun 16, 2016

I agree; I have never liked rcParams, with its massive global state. It is particularly dangerous as a mechanism for controlling local state, flipping settings back and forth. I suspect we can figure out better ways of handling this, but not as modifications to 2.0.

@dopplershift
Copy link
Contributor Author

Well, we have a problem for 2.0 since the stylesheets will IMO see a lot more use as people fall back to 'classic' for cases like this. The fact that our own docs are wrong on this really bothers me.

@tacaswell
Copy link
Member

@dopplershift it definitely should work as you expected.

@efiring
Copy link
Member

efiring commented Jun 16, 2016

I'm probably over-reacting because of my dislike for rcParams; I will defer to @tacaswell's judgment. It seems like the question is whether all factors influenced by rcParams can be frozen based on the rcParams values at the time of artist creation, and the answer is "no", because layout is only finalized when the figure is drawn, so it has to depend on figure.dpi at that point. Properties like color are easy to handle; anything involving layout is not. Even though they involve size, I think the dash patterns should be in the former (easy) category, and if they are not, that is a bug. (I have a vague sense that we have dpi dependencies where we shouldn't--things are getting converted back and forth between dots and physical units unnecessarily.)

@tacaswell
Copy link
Member

with #6710

You get
test1
test2
test3

I do not think you can get around the remaining difference between test1 and test2 because the default savefig dpi should be looked up at save time, not at figure creation time.

tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jul 12, 2016
When scaling the dash pattern by the linewidth do the scaling at artist
creation / value set time rather than at draw time.

closes matplotlib#6592
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jul 12, 2016
When scaling the dash pattern by the linewidth do the scaling at artist
creation / value set time rather than at draw time.

closes matplotlib#6592
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jul 12, 2016
When scaling the dash pattern by the linewidth do the scaling at artist
creation / value set time rather than at draw time.

closes matplotlib#6592
closes matplotlib#6588
closes matplotlib#6590
Closes matplotlib#6693
closes matplotlib#5430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants