Skip to content

TST: Use explicit style in all image_comparison calls#31148

Merged
QuLogic merged 1 commit intomatplotlib:mainfrom
QuLogic:image-comparison-style
Apr 17, 2026
Merged

TST: Use explicit style in all image_comparison calls#31148
QuLogic merged 1 commit intomatplotlib:mainfrom
QuLogic:image-comparison-style

Conversation

@QuLogic
Copy link
Copy Markdown
Member

@QuLogic QuLogic commented Feb 13, 2026

PR summary

This makes it so that future tests must use the new style (or explicitly opt in to the old), and in the future, we may return to a default of that one instead.

I used a libcst transform to add the kwarg in alphabetical order, so some bits of the diff might be suboptimal in their succinctness, but they should be all consistent.

Fixes #24716

This is based on #31137.

PR checklist

@QuLogic QuLogic mentioned this pull request Feb 13, 2026
Closed
@QuLogic QuLogic force-pushed the image-comparison-style branch from 5ac6017 to 85a4a72 Compare February 14, 2026 21:22
timhoffm
timhoffm reviewed Feb 15, 2026
View reviewed changes
Comment thread lib/matplotlib/tests/test_agg.py Outdated

@image_comparison(['agg_filter.png'], remove_text=True)
@image_comparison(['agg_filter.png'], remove_text=True,
style=('classic', '_classic_test_patch'))
Copy link
Copy Markdown
Member

@timhoffm timhoffm Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create a new style _classic_test? This feels overly verbose.

Copy link
Copy Markdown
Member Author

@QuLogic QuLogic Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that we have a way to create a kind of inheritance that would work like that. If we made a completely new one, we'd be duplicating most of classic with the 2 or 3 lines of _classic_test_patch differing.

Copy link
Copy Markdown
Member

@timhoffm timhoffm Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we dynamically create _classic_test.mplstyle by merging the two files at test startup? Or check it in and write a test that it's just the merge of the two?

Copy link
Copy Markdown
Member Author

@QuLogic QuLogic Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the former seems quite plausible.

Copy link
Copy Markdown
Member

@timhoffm timhoffm Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuLogic I thought about how to document this for debugging (=I want to manually create the same image as in a test). We basically have to state: When running manually, use ('classic', '_classic_test_patch') instead of '_classic_test'.

That brought me to the idea that instead of doing magic at test setup, we could alternatively rewrite the value '_classic_test' to ('classic', '_classic_test_patch'). Not sure which one is better.

Copy link
Copy Markdown
Member Author

@QuLogic QuLogic Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's more than just the style; we do a few other things as part of test setup:

matplotlib/lib/matplotlib/testing/__init__.py

Lines 30 to 52 in cb63559

def setup():
# The baseline images are created in this locale, so we should use
# it during all of the tests.
try:
locale.setlocale(locale.LC_ALL, 'en_US.UTF-8')
except locale.Error:
try:
locale.setlocale(locale.LC_ALL, 'English_United States.1252')
except locale.Error:
_log.warning(
"Could not set locale to English/United States. "
"Some date-related tests may fail.")
mpl.use('Agg')
with _api.suppress_matplotlib_deprecation_warning():
mpl.rcdefaults() # Start with all defaults
# These settings *must* be hardcoded for running the comparison tests and
# are not necessarily the default values as specified in rcsetup.py.
set_font_settings_for_testing()
set_reproducibility_for_testing()

The best way to create a test image is by running an image_comparison and looking at the result image.

@QuLogic QuLogic force-pushed the image-comparison-style branch from 85a4a72 to a18b014 Compare February 25, 2026 07:22
@QuLogic QuLogic force-pushed the image-comparison-style branch from a18b014 to ec3a2df Compare February 25, 2026 07:30
@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Mar 11, 2026

Note, I've decided to wait until the text-overhaul branch is merged before updating this one, as it will change a lot of tests to use mpl20.

@QuLogic QuLogic force-pushed the image-comparison-style branch from ec3a2df to 3f4bc2c Compare April 10, 2026 19:47
@QuLogic QuLogic force-pushed the image-comparison-style branch from 3f4bc2c to de1e5d8 Compare April 10, 2026 19:58
@QuLogic QuLogic added this to the v3.11.0 milestone Apr 10, 2026
@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Apr 10, 2026

Sorry, followup question, is image_comparison public API? Should I write a release note about the style parameter, use our deprecation machinery, or revert that part entirely now that we know all tests have been tagged?

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Apr 12, 2026

https://github.com/search?q=matplotlib.testing+.decorators+language%3APython&type=code This search says even if we did not want it to be, it is.

I would proprose

  1. document it as an API change
  2. set the default to None, warn if not set, and then remain at the previous default
  3. in 3.13 we move it to optional keyword only defaulting to 'mpl20'

@timhoffm timhoffm mentioned this pull request Apr 13, 2026
Closed
6 tasks
@QuLogic QuLogic force-pushed the image-comparison-style branch 3 times, most recently from 45440d3 to 7207881 Compare April 13, 2026 19:13
@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Apr 13, 2026

Funnily enough, this one overrides ours to set mpl20 as the default.

tacaswell
tacaswell reviewed Apr 16, 2026
View reviewed changes
Comment thread lib/matplotlib/testing/decorators.py Outdated
@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Apr 16, 2026

@QuLogic can self-merge after fixing the docs/linting issue.

@QuLogic
TST: Use explicit style in all image_comparison calls
47f5e22 
Due to the warning and our default to set warnings to errors, this makes
it so that future tests *must* use the new style (or explicitly opt in
to the old), and in the future, we will return to a default of that one
instead.

As a shortcut, the `_classic_test` style is generated from `classic` and
`_classic_test_patch` at test startup.

Fixes matplotlib#24716
@QuLogic QuLogic force-pushed the image-comparison-style branch from c03b703 to 47f5e22 Compare April 16, 2026 19:38
@QuLogic QuLogic merged commit 5d0a4f5 into matplotlib:main Apr 17, 2026
41 checks passed
@QuLogic QuLogic deleted the image-comparison-style branch April 17, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

topic: testing

Projects

None yet

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

[TST]: Add classic style to all old image tests.

5 participants

@QuLogic @tacaswell @timhoffm @melissawm @dstansby