Skip to content

Fix themeable._omit inheritance - #1056

Merged
has2k1 merged 1 commit into
mainfrom
themeable-inheritance
Apr 30, 2026
Merged

Fix themeable._omit inheritance#1056
has2k1 merged 1 commit into
mainfrom
themeable-inheritance

Conversation

@has2k1

@has2k1 has2k1 commented Apr 30, 2026

Copy link
Copy Markdown
Owner

_omit was a class attribute consulted by themeable.properties to strip keys that mpl.set could not accept on the artist. Because the themeable hierarchy is inverted — general themeables are Python subclasses of specifics — Python's MRO attribute lookup made a general themeable (rect, axis_text, line) silently inherit _omit from its first specific base. That filter, calibrated for one specific's artist, was then applied to every artist the general themeable composed via super() chaining.

The bug was masked for users by the merge cascade in Themeables.update, but visible under direct themeable.apply (tests, extensions) and latent for any future reader of instance.properties outside apply_*. By happenstance, it was also masked by the default values used to create themes: e.g. when defining theme_gray, if you change the rect to have fill="pink" it would not apply!, but with "white" it went unnoticed.

Replace _omit with themeable._get_properties(omit=...) called inside each apply_* method. The filter is local to the apply call, tied to the mpl_artist.set(**) it feeds, and not subject to MRO attribute leakage. Three previously-defensive
with suppress(KeyError): del props[KEY] blocks in the title themeables (axis_title_x ha, axis_title_y va, plot_title ha) were also unconditional drops that fold cleanly into the helper call.

Composition fix: when the bug was masked, the composition's outer plot_background was incidentally transparent. With the fix it correctly carries rect.fill, exposing a latent layering issue — at zorder=-1000 the outer covered the per-plot tinted backgrounds at the same zorder. Move the composition outer to zorder=-2000 so per-plot backgrounds layer above it.

`_omit` was a class attribute consulted by `themeable.properties` to
strip keys that mpl.set could not accept on the artist. Because the
themeable hierarchy is inverted — general themeables are Python
subclasses of specifics — Python's MRO attribute lookup made a
general themeable (`rect`, `axis_text`, `line`) silently inherit
`_omit` from its first specific base. That filter, calibrated for one
specific's artist, was then applied to every artist the general
themeable composed via super() chaining.

The bug was masked for users by the merge cascade in
`Themeables.update`, but visible under direct `themeable.apply`
(tests, extensions) and latent for any future reader of
`instance.properties` outside `apply_*`. By happenstance, it was also
masked by the default values used to create themes: e.g. when defining
`theme_gray`, if you change the `rect` to have `fill="pink"` it would
not apply!, but with "white" it went unnoticed.

Replace `_omit` with `themeable._get_properties(omit=...)` called
inside each `apply_*` method. The filter is local to the apply call,
tied to the `mpl_artist.set(**)` it feeds, and not subject to MRO
attribute leakage. Three previously-defensive
`with suppress(KeyError): del props[KEY]` blocks in the title
themeables (axis_title_x ha, axis_title_y va, plot_title ha) were
also unconditional drops that fold cleanly into the helper call.

Composition fix: when the bug was masked, the composition's outer
`plot_background` was incidentally transparent. With the fix it
correctly carries `rect.fill`, exposing a latent layering issue —
at zorder=-1000 the outer covered the per-plot tinted backgrounds
at the same zorder. Move the composition outer to zorder=-2000 so
per-plot backgrounds layer above it.
@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.81%. Comparing base (781f680) to head (b105dec).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1056      +/-   ##
==========================================
- Coverage   86.84%   86.81%   -0.03%     
==========================================
  Files         203      203              
  Lines       13788    13759      -29     
  Branches     1689     1689              
==========================================
- Hits        11974    11945      -29     
  Misses       1265     1265              
  Partials      549      549              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@has2k1
has2k1 merged commit b105dec into main Apr 30, 2026
14 checks passed
@has2k1
has2k1 deleted the themeable-inheritance branch April 30, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant