TST: Remove unnecessary test images #29827
Conversation
|
If code coverage passes, that should be at least some indicator that we didn't drop anything relevant. |
|
OK, I've also removed |
|
Why prefer png over svg/pdf? A spot check of sizes looks like the pdf's are the smallest (presumably because they are compressed by default) and svgs are the most "text like" so should play the best with git. I was going to assert that the vector layouts would be insensative to freetype but began to doubt that as I was typing (I am not sure if we rely on freetype to do any placement even in those cases). The biggest upside I see of png is that it is no optional test-deps to run. |
Unfortunately not, or this wouldn't have come up in #29816. Admittedly, I don't think it's all of them, though.
This is definitely the main reason, but I have no issues with changing to SVG/PDF if we think that's okay. |
|
I also suspect (but have not measured) that the png tests are faster as we don't effectively render 2x and require a sub-process to be running. Despite asking the question, I'm coming around to png being the right choice. |
|
Should we change the default extensions of check_figures_equal to only png? Figure comparisons often create the same artists but with different code paths. Therefore rendering to any output should be sufficient. I believe it's quite the exception that we need more than one output. |
|
Good idea, but it is in the public API right now. I seem to recall us saying that |
|
I'd be so bold and just change it, documenting it in an API change note. It should be "what users want" 99% of the time, and there's no simple deprecation/migration route. |
|
OK, I've changed the default and removed the extraneous |
| @@ -677,8 +677,7 @@ def test_contains_points(): | |||
| assert np.all(result == expected) | |||
|
|
|||
|
|
|||
| # Currently fails with pdf/svg, probably because some parts assume a dpi of 72. | |||
There was a problem hiding this comment.
It looks like it does still fail on PDF/SVG, so we can leave it in.
|
Not a 100% exact comparison but this test run is 9630 tests in 12:37min, where a test run on another recent PR is 9845 tests in 13:54min. So a rough estimate is that we have shaved off 200 tests and 1:20min test runtime. That's a significant improvement 🎉. |
|
This and #29816 will need to be backported to 3.10.x. |
lib/matplotlib/tests/test_figure.py
Outdated
| @@ -25,7 +25,7 @@ | |||
| import matplotlib.dates as mdates | |||
|
|
|||
|
|
|||
| @image_comparison(['figure_align_labels'], extensions=['png', 'svg'], | |||
| @image_comparison(['figure_align_labels.png'], | |||
There was a problem hiding this comment.
Sure, can revert. That commit also has some backcompat stuff that maybe could be removed in #29816, looks like...
There's no need to test multiple file formats for tests that: - are for wrappers that simplify or combine other primitives, such as `boxplot`, `eventplot`, `axhspan`, or `fill_between`. - are for spine or tick settings, such as the `autoscale_tiny_range`, `formatter_ticker_*`. - are for alternate Axes projections, such as polar, Mollweide, or skewed Axes.
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
…3.10.x Backport PR #29827 on branch v3.10.x (TST: Remove unnecessary test images)
PR summary
There's no need to test multiple file formats for tests that:
boxplot,eventplot,axhspan, orfill_between.autoscale_tiny_range,formatter_ticker_*.This is certainly not exhaustive; I did not remove any of the mathtext formats (seemed like a good glyph test), anything with hatching/clipping/alpha (probably could be pared down to just a few),
markevery tests (don't recall if that required anything in the backend),or tight layout tests.PR checklist