-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Creation of the 'classic' matplotlib style #4669
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
also still needs docs. |
sortof blocked by #4218 |
This PR is now based on the other validation PR. I have also updated this PR to base all unit tests on the new 'classic' mode. Also added a special 'default' style. |
Ok, managed to get a few more tests passing (apparently, some of them were very sensitive to path simplification thresholds of 0.1 versus 0.111111111). I have found a bug with the image comparison class that I exacerbated. If the test modifies the rc params as part of the test, and those rc params do not take effect until render time (i.e., path simpliciation, dpi, etc.), then the context manager in which we establish the default style on all tests nukes those changes since the savefig step happens outside the context manager. I am not sure what to do about that. |
@WeatherGod, I think you have hit upon an issue that has been worrying me with respect to the plans to redo the whole rcParams/configuration/property/traitlets business: there are quite a few places where some things are now set at object creation time, others at draw time; where things get modified based on initial drawing; etc. The possibilities for breakage and running into gotchas are extensive. This is going to be hard to sort out. |
@efiring, this is very true, and I have no delusions that the whole traitlets/serialization business will be easy. But if matplotlib is to grow, we need a better system to build it upon, so I would rather take the pain now. As for the problem at hand, perhaps the solution is fairly simple... don't use a context manager. Instead, I should probably hook onto the setup/cleanup mechanisms and manage the style handling that way. |
Making progress with this approach. Can anyone tell me why I am getting errors trying to find the font family 'sans-serif'? I have also tried 'sans-serif' to no avail. |
Well, looks like everything passes on Travis, even though it fails miserably locally for me. That's not exactly warm fuzzies for me... I'll work on adding documentation tonight. |
Just did a rebase and incorporated the recent feature to specify inverse functions for deprecated rcparams. I'll update later for any new rc params and write up docs. |
...and I am getting my PRs mixed up. The previous comment was meant for my property-cycler PR. This one has also been rebased, but needs the new params and documentation. |
huh, now a lot of stuff fails on Travis. A bunch of it relating to tight_layout and others related to stix fonts. I suspect the tight_layout problems really are font problems in disguise. Maybe I have my font specifications all wrong in the classic style? |
I am getting the following warning in Travis:
This is probably the source of my font problems. But I am not clear how my 'classic' style is any different from the defaults. |
I think the message:
is a red herring. It's emitted on master as well, and I think has to do with a test writing explicitly to the PostScript backend with embedded fonts only turned on. (I should probably be in a I'll push your branch to a branch on the main fork so that we can get a downloadable image packet. |
Here's the Travis build for the temporary branch: https://travis-ci.org/matplotlib/matplotlib/builds/73086282 |
Thanks @mdboom. I'll take a good look at the results tomorrow. In the meantime, I just added some preliminary documentation about this feature in "whats new". I noticed that it doesn't seem like any of the other new styles are mentioned. It got me thinking that it might be nice to have a page that showcases the available styles that matplotlib ships with? |
@tonysyu has some code for making a style gallery someplace. |
And the result images (from the Python 3.4 build, if that matters) are now available here: https://s3.amazonaws.com/matplotlib-test-results/artifacts/7686/7686.4/result_images.tar.bz2 |
Looking at the images: For the SVG ones, both the baseline and generated images are using Bitstream Vera Sans, but there does seem to be some moving around of objects everywhere. For the PNG ones, the baseline images seem to have Helvetica (or maybe Arial), whereas the generated ones are Bitstream Vera Sans. So this suggests to me that something is really funky with the font lookup and maybe the baseline images were generated with the wrong fonts. I'll see if I can reproduce the failure locally to confirm. |
Well, more specifically, this is happening with my classic mode stuff, On Tue, Jul 28, 2015 at 5:33 PM, Michael Droettboom <
|
Ok, now trying to get down and dirty to make sure there aren't unnoticed differences:
When I diff mpl_defaults.txt with mpl_origs.txt, I just get a difference in default backends, which is expected. When I diff mpl_defaults.txt with mpl_classic.txt, I get the following:
So, it looks like I need to correct the path.effects parameter, but I think all the others are safe to exclude from the classic mode file. |
@WeatherGod: I'm sort of driveby commenting here, so ignore me if I'm out of date. Are we still having font match issues here? You may want to try #4689. |
I was just thinking that as I was reading those comments. I'll give it a On Thu, Aug 6, 2015 at 4:54 PM, Michael Droettboom <notifications@github.com
|
sigh... does not appear to be the case. |
Interesting observation... If I change the default style used in the testing decorator to be 'default', then I get the same failures. So the problem isn't caused by the classic mode, per-se. If I take out the style-handling in the setup/teardown code, then things are back to normal (but classic mode is never applied for the tests). |
Curiouser and curiouser... if I don't use "matplotlib.rcParams.update()" at all for the tests, and run with --verbose-debug, I get a single findfont message:
But, if I use "matplotlib.rcParams.update()", then I get two findfont messages, the first one the same as above, and the second one slightly different:
|
and I can no longer reproduce that double-findfont behavior... This is driving me insane! I can't be 100% sure that this is a font problem, per-se. Looking closely at the test failure for mplot3d's text3d test, the z-axis line is not drawn quite right (it is very close to vertical, but not quite). |
So now I tried running the text3d test completely separately from any test framework and generate an image from that (no applying of any particular styles). Now, since I have been able to run the tests in such a way that they pass (by completely turning off the apply/restore of styles for each test), I would expect that this image be identical to the baseline image, but it isn't! It was identical to the failed image! This makes me wonder if there is something "wrong" with the testing framework that is applying some sort of setting that is carrying over from one test to the next, and that my patch "fixes" that? Note that test_mplot3d.py does not set any rcparams, so it isn't the tests themselves that is causing the problem (I don't think). Perhaps we have a wayward rcparam assignment somewhere in our codebase, and I am undoing it at the end of each test? |
Aha! There is code somewhere that is modifying the rcparams. I did the following to the teardown function:
(done this way because nose kept truncating the assertion's message). I got the following result upon teardown of the second test for test_mplot3d (and after the third test for test_axes_grid1.py):
So, somehow, text hinting is getting turned off, and it stays off for the rest of the tests. The font.family being switched to "Bitstream Vera Sans" is probably what is throwing off the stix tests, I'd imagine. |
And I have come around to the source of the issue. In |
Looks like all I need to do is move that call to |
Close, but no cigar. We need to do param cleanup in the cleanup decorator as well. By the way, I think I have found the source of the bug that caused the intermittent latex failures! There is likely a race condition because the testing is being done in two threads, and the tests related to the latex failures heavily depend upon modifying the rcparams on-the-fly. In particular, setting "usetex" to True. Well, if "usetex" gets unset prior to calling the savefig for that test, then latex wouldn't get called, and the stringio comparison function would just sit there, waiting for something that will never come. |
I thought the tests ran as separate processes, not threads. |
That's what I thought, and maybe I am wrong. It is something to keep in mind for further investigation. |
Woot! The tests pass! Someone should probably take a peek at the bastardization I did to the testing framework, but it seems like it works! |
ENH: Creation of the 'classic' matplotlib style
👏 |
Dependent upon #4647. Also not fully tested and vetted. I need to see if I missed any params.