Pdf color none #8850
Pdf color none #8850
Conversation
…rison and unit test case
| @@ -191,3 +193,17 @@ def psfont(*args, **kwargs): | |||
| ax.text(0.5, 0.5, 'hello') | |||
| with tempfile.TemporaryFile() as tmpfile, pytest.raises(ValueError): | |||
| fig.savefig(tmpfile, format='pdf') | |||
|
|
|||
|
|
|||
| def test_pdf_savefig_when_color_is_none(tempdir_factory): | |||
There was a problem hiding this comment.
The simpler tmpdir would be better here; don't see why it needs a factory.
There was a problem hiding this comment.
I don't even see where tempdir_factory is defined? (even by pytest itself?)
There was a problem hiding this comment.
Probably a typo, which is why it's not passing; should have been tmpdir_factory.
|
|
||
|
|
||
| def test_pdf_savefig_when_color_is_none(tempdir_factory): | ||
| rcParams['_internal.classic_mode'] = False |
There was a problem hiding this comment.
I don't know what this entails? It's neither classic nor default style now.
|
|
||
|
|
||
| @pytest.mark.style('default') | ||
| def test_pdf_savefig_when_color_is_none(tmpdir_factory): |
There was a problem hiding this comment.
The tmpdir fixture is simpler...
There was a problem hiding this comment.
but then I have to sort out how to use it....
There was a problem hiding this comment.
Just drop the .mktemp() since it's doing that for you.
b37f142 to
51f0705
Compare
|
Squashed commits, simplified tests, and changed to use public API instead of private attributes. |
| @@ -355,8 +355,8 @@ def crop_to_same(actual_path, actual_image, expected_path, expected_image): | |||
| # clip the images to the same size -- this is useful only when | |||
| # comparing eps to pdf | |||
| if actual_path[-7:-4] == 'eps' and expected_path[-7:-4] == 'pdf': | |||
| aw, ah = actual_image.shape | |||
| ew, eh = expected_image.shape | |||
| aw, ah, ad = actual_image.shape | |||
There was a problem hiding this comment.
What happened here? How would this have worked before?
There was a problem hiding this comment.
|
I am 👍 on merging this, but can not review a PR I opened (which sorta makes sense). Anyone else who approves this should merge it. |
|
@Tuan333 Thanks for you work on this! |
Replaces #8674