diff --git a/plotnine/_utils/yippie.py b/plotnine/_utils/yippie.py index ab8f44f50..cbc374626 100644 --- a/plotnine/_utils/yippie.py +++ b/plotnine/_utils/yippie.py @@ -40,7 +40,6 @@ def __getattr__(self, color: str): title=color.title(), ) + theme( - figure_size=(8, 6), text=element_text(color="black", size=11), panel_background=element_rect(fill=color, size=1), plot_background=element_rect(fill=color, alpha=0.2), diff --git a/plotnine/composition/_inset_image.py b/plotnine/composition/_inset_image.py index 2df9e66e3..837f63467 100644 --- a/plotnine/composition/_inset_image.py +++ b/plotnine/composition/_inset_image.py @@ -123,7 +123,7 @@ def _arrange_in_box( self.figure, anchor=self._anchor, ) - self._frac_bbox.bounds = (l, b, r - l, t - b) # pyright: ignore[reportAttributeAccessIssue] + self._frac_bbox.bounds = (l, b, r - l, t - b) self.patch.set_bounds(left, bottom, right - left, top - bottom) # The layout engine has finalised the bbox, so its device-pixel diff --git a/plotnine/geoms/geom_ribbon.py b/plotnine/geoms/geom_ribbon.py index b28527f31..0d305b852 100644 --- a/plotnine/geoms/geom_ribbon.py +++ b/plotnine/geoms/geom_ribbon.py @@ -142,7 +142,7 @@ def draw_unit( _x, _min, _max, - where=where, # type: ignore + where=where, interpolate=interpolate, facecolor=fill, edgecolor=color, diff --git a/pyproject.toml b/pyproject.toml index 0a418dd01..c9d84149d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -88,7 +88,7 @@ dev = [ ] typing = [ - "pyright==1.1.410", + "pyright==1.1.411", "ipython", "pandas-stubs", ] @@ -207,6 +207,14 @@ stubPath = "" ########## Tool - Ruff ########## [tool.ruff] line-length = 79 +# Exclude a variety of commonly ignored directories. +exclude = [ + "plotnine/themes/seaborn_rcmod.py", + "**/__pycache__", + "node_modules", + "README.md", +] + [tool.ruff.lint] select = [ @@ -241,14 +249,6 @@ ignore = [ fixable = ["ALL"] unfixable = [] -# Exclude a variety of commonly ignored directories. -exclude = [ - "plotnine/themes/seaborn_rcmod.py", - "**/__pycache__", - "node_modules" -] - - # Allow unused variables when underscore-prefixed. dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" diff --git a/tests/baseline_images/test_layout/plot_margin_aspect_ratio.png b/tests/baseline_images/test_layout/plot_margin_aspect_ratio.png index b63149ecf..3d58ae662 100644 Binary files a/tests/baseline_images/test_layout/plot_margin_aspect_ratio.png and b/tests/baseline_images/test_layout/plot_margin_aspect_ratio.png differ diff --git a/tests/conftest.py b/tests/conftest.py index 51f717086..0961fe8e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ inset_element, plot_annotation, ) +from plotnine.options import set_option from plotnine.themes.theme import DEFAULT_RCPARAMS TOLERANCE = 2 # Default tolerance for the tests @@ -28,11 +29,14 @@ DEFAULT_RCPARAMS["font.sans-serif"] = ["Dejavu Sans"] DEFAULT_RCPARAMS["font.serif"] = ["Dejavu Serif"] -# This partial theme modifies all themes that are used in -# the test. It is limited to setting the size of the test -# images Should a test require a larger or smaller figure -# size, the dpi or aspect_ratio should be modified. -test_theme = theme(figure_size=(640 / DPI, 480 / DPI), dpi=DPI) +# Set figure size and DPI through package options so plot themes can override +# them. Tests that omit either setting retain a 640-by-480-pixel image at +# 72 DPI. Configure the options before test modules construct their themes. +set_option("dpi", DPI) +set_option("figure_size", (640 / DPI, 480 / DPI)) + +# Give compositions and standalone insets more room than a single plot. +default_composition_theme = theme(figure_size=(8, 6), dpi=DPI) tests_dir = Path(__file__).parent baseline_images_dir = tests_dir / "baseline_images" @@ -70,7 +74,6 @@ def ggplot_equals(plot: ggplot, name: str) -> bool: # Save the figure before testing whether the original image # actually exists. This makes creating new tests much easier, # as the result image can afterwards just be copied. - plot += test_theme with _test_cleanup(): plot.save(filenames.result, verbose=False) @@ -240,7 +243,22 @@ def composition_equals(cmp: Compose, name: str) -> bool: test_file = inspect.stack()[1][1] filenames = make_test_image_filenames(name, test_file) - _cmp = cmp + plot_annotation(theme=theme(figure_size=(8, 6), dpi=DPI)) + # Apply the test default after any complete theme broadcast with `&`. Its + # default figure size is indistinguishable from an explicit choice. + # Preserve a size set on a partial annotation theme because it explicitly + # sizes the composition. + annotation_theme = cmp.annotation.theme + declares_own_size = ( + not annotation_theme.complete + and annotation_theme.getp("figure_size") is not None + ) + composition_theme = ( + default_composition_theme + annotation_theme + if declares_own_size + else default_composition_theme + ) + + _cmp = cmp + plot_annotation(theme=composition_theme) with _test_cleanup(): _cmp.save(filenames.result) @@ -286,7 +304,7 @@ def inset_element_equals(inset: inset_element, name: str) -> bool: test_file = inspect.stack()[1][1] filenames = make_test_image_filenames(name, test_file) - host = inset._blank_host + theme(figure_size=(8, 6), dpi=DPI) + host = inset._blank_host + default_composition_theme with _test_cleanup(): (host + inset).save(filenames.result, verbose=False) diff --git a/tests/test_layout.py b/tests/test_layout.py index 6acd8dc5c..ed4099c7c 100644 --- a/tests/test_layout.py +++ b/tests/test_layout.py @@ -136,7 +136,7 @@ def test_plot_margin_aspect_ratio(self): p = ( ggplot() + geom_blank() - + theme(plot_margin=0.025, figure_size=(4, 3)) + + theme(plot_margin=0.025, figure_size=(8, 4)) ) assert p == "plot_margin_aspect_ratio" diff --git a/tools/visualize_tests.py b/tools/visualize_tests.py index c5ba906f5..eca724a14 100644 --- a/tools/visualize_tests.py +++ b/tools/visualize_tests.py @@ -7,6 +7,7 @@ # import argparse +import struct from dataclasses import dataclass from pathlib import Path from typing import Iterator @@ -41,6 +42,20 @@ def highlight_python(source: str) -> str: return escape(source) +def png_size(path: Path) -> tuple[int, int] | None: + """ + Read the pixel width and height of a PNG file + + Returns `None` if the file is not a PNG or is truncated. + """ + with path.open("rb") as f: + header = f.read(24) + if len(header) < 24 or header[:8] != b"\x89PNG\r\n\x1a\n": + return None + width, height = struct.unpack(">II", header[16:24]) + return width, height + + @dataclass(frozen=True) class TestImage: """A single image-comparison test result""" @@ -50,10 +65,23 @@ class TestImage: actual_rel: str expected_rel: str | None failed_rel: str | None + actual_size: tuple[int, int] | None + expected_size: tuple[int, int] | None + + @property + def size_mismatch(self) -> bool: + """Whether the two images have different pixel dimensions""" + return ( + self.actual_size is not None + and self.expected_size is not None + and self.actual_size != self.expected_size + ) @property def status(self) -> str: - if self.failed_rel: + # The test framework compares pixels, so differently sized images + # fail without producing a diff image. + if self.failed_rel or self.size_mismatch: return "failed" if self.expected_rel is None: return "new" @@ -98,6 +126,10 @@ def get_test_images() -> Iterator[TestImage]: failed_rel=( f"{subdir.name}/{failed.name}" if failed.exists() else None ), + actual_size=png_size(png), + expected_size=( + png_size(expected) if expected.exists() else None + ), ) @@ -459,6 +491,35 @@ def get_test_images() -> Iterator[TestImage]: border-radius: 4px; } +.images figcaption .dims { + text-transform: none; + letter-spacing: 0; + font-variant-numeric: tabular-nums; +} + +/* Scaling both images to the column width would hide the very + difference this view exists to show. */ +.images-natural figure { + flex: 0 0 auto; + min-width: 0; + max-width: 100%; +} + +.images-natural img { + width: auto; + max-width: 100%; +} + +.note { + margin: 0; + padding: 6px 10px; + border: 1px solid var(--red); + border-radius: 6px; + background: var(--red-soft); + color: var(--red); + font-size: 12px; +} + .test-row.view-side .images-slider, .test-row.view-side .images-flip, .test-row.view-slider .images-side, @@ -923,7 +984,7 @@ def get_test_images() -> Iterator[TestImage]: const cap = fig.querySelector('figcaption'); if (img && cap) { sibs.push({ - label: cap.textContent.trim(), + label: fig.dataset.label || cap.textContent.trim(), src: img.getAttribute('src'), }); } @@ -1103,14 +1164,43 @@ def get_test_images() -> Iterator[TestImage]: """ -def _img_cell(caption: str, src: str) -> str: +def _img_cell( + caption: str, src: str, size: tuple[int, int] | None = None +) -> str: + dims = f' {size[0]}×{size[1]}' if size else "" return ( - f"
{caption}
" + f'
' + f"
{caption}{dims}
" f'' f"
" ) +def _size_mismatch_content(test: TestImage) -> str: + """ + Content for a test whose images cannot be compared pixel-by-pixel + + Both images are shown at their natural size, so the difference in + dimensions is visible rather than scaled away. + """ + assert test.actual_size is not None + assert test.expected_size is not None + aw, ah = test.actual_size + ew, eh = test.expected_size + note = ( + f'

Image sizes differ: actual {aw}×{ah}, ' + f"expected {ew}×{eh}. There is no pixel diff.

" + ) + return ( + '
' + + note + + '
' + + _img_cell("actual", test.actual_rel, test.actual_size) + + _img_cell("expected", test.expected_rel or "", test.expected_size) + + "
" + ) + + def _failed_content(test: TestImage) -> str: actual = test.actual_rel expected = test.expected_rel or "" @@ -1251,7 +1341,9 @@ def render_row(test: TestImage) -> str: ) meta = '
' + "".join(parts) + "
" - if test.status == "failed": + if test.size_mismatch: + content = _size_mismatch_content(test) + elif test.status == "failed": content = _failed_content(test) elif test.status == "new": content = _new_content(test)