-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Update docs for writing image comparison tests. #12552
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
Prefer directing to the function's docstring as the main source of truth -- testing.rst had some info that was invalid since a while ago, namely regarding the default value of tol. Also replace the example image comparison test by a much shorter one.
Looks like the overall information is still useful at that place. Maybe the exact image types and tolerance can simply be left out, such that this section would not require any updates. And then linking to the docstrings via "See ``.path.to.doc` ` for the exact signature and possible argument values."? Also actually mentionning the fact that the tolerance is subject to change depending on new versions of inkscape (or whatever is responsible) is a useful piece of information. |
My problem is that it's not just the default that became wrong, it's also that we rely on |
doc/devel/testing.rst
Outdated
from matplotlib.testing.decorators import image_comparison | ||
import matplotlib.pyplot as plt | ||
|
||
@image_comparison(baseline_images=['hexbin_empty'], remove_text=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use another test please? This is more on the obscure side. It may not be immediately evident why we test an empty plot (and why does it have to be an image comparison test?). Also, the comment may be misleading, because it does not describe the test, but a bug that the test verifies to be fixed.
If you want a real-world example, how about:
@image_comparison(baseline_images=['line_dashes'], remove_text=True)
def test_line_dashes():
fig, ax = plt.subplots()
ax.plot(range(10), linestyle=(0, (3, 3)), lw=5)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add extensions
if you wish.
lib/matplotlib/testing/decorators.py
Outdated
@@ -369,9 +369,14 @@ def image_comparison(baseline_images, extensions=None, tol=0, | |||
only allowed when using pytest. | |||
|
|||
extensions : [ None | list ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it None or list of str
if you like.
Please push the changes directly, they look fine to me. |
ax.hexbin([], []) | ||
def test_line_dashes(): | ||
fig, ax = plt.subplots() | ||
ax.plot(range(10), linestyle=(0, (3, 3)), lw=5) | ||
|
||
The first time this test is run, there will be no baseline image to compare | ||
against, so the test will fail. Copy the output images (in this case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timhoffm the paths below need to be updated as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
490d2bc
to
96f97fa
Compare
Prefer directing to the function's docstring as the main source of
truth -- testing.rst had some info that was invalid since a while ago,
namely regarding the default value of tol.
Also replace the example image comparison test by a much shorter one.
PR Summary
PR Checklist