Skip to content

Add colorblind-safe visualization theme and resolve merge conflict #136

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.

Sign up for GitHub

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

Open
Software566 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add colorblind-safe visualization theme and resolve merge conflict #136

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6667cb3
Add colorblind-safe visualization theme
Jun 13, 2026
424f1d2
Add colorblind-safe visualization theme
Jun 13, 2026
2112d87
Resolve merge conflict in trustlens/visualization/style.py
Jun 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 44 additions & 87 deletions trustlens/visualization/style.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@

from __future__ import annotations

import warnings
from collections.abc import Iterator
from copy import deepcopy
from contextlib import contextmanager
from dataclasses import dataclass, field
from typing import Any
import warnings

import matplotlib as mpl
import matplotlib.pyplot as plt
Expand Down Expand Up @@ -157,6 +158,25 @@
"bar_alpha": 0.85,
}

COLORBLIND_PALETTE: list[str] = [
"#0072B2",
"#E69F00",
"#009E73",
"#CC79A7",
"#56B4E9",
"#D55E00",
"#F0E442",
]

COLORBLIND_SEMANTIC_COLORS: dict[str, dict[str, str]] = {
"severity": {
"acceptable": "#0072B2",
"moderate": "#E69F00",
"severe": "#D55E00",
"unknown": "#999999",
}
}

# ---------------------------------------------------------------------------
# Theme — frozen dataclass bundling everything above for future extensibility
# ---------------------------------------------------------------------------
Expand All @@ -171,28 +191,6 @@ class Theme:
that future themes (``"dark"``, ``"colorblind"``, ``"publication"``) can be
added by registering a new :class:`Theme` instance without touching
plotting code.

Attributes
----------
name : str
Identifier (e.g. ``"default"``).
base_style : str
Matplotlib base style sheet applied inside :func:`apply_style`.
palette : list[str]
Categorical palette as ordered hex strings.
brand : dict[str, str]
Named brand colors.
semantic : dict[str, dict[str, str]]
Semantic color groups (``severity``, ``verdict``, ``grade``,
``direction``, ``neutral``).
typography : dict[str, Any]
Font family and size settings.
grid : dict[str, Any]
Grid alpha and linewidth.
fig_defaults : dict[str, Any]
Default figure size, facecolor, and DPI.
spacing : dict[str, Any]
Padding, alpha, and bar styling.
"""

name: str = "default"
Expand All @@ -210,6 +208,12 @@ class Theme:

DEFAULT_THEME: Theme = Theme()

COLORBLIND_THEME: Theme = Theme(
name="colorblind",
palette=list(COLORBLIND_PALETTE),
semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS),
)
Comment on lines +211 to +215

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing semantic color mappings will cause KeyError at runtime.

COLORBLIND_SEMANTIC_COLORS only defines the "severity" key (lines 171-178), but the default SEMANTIC_COLORS provides five semantic mappings: "severity", "verdict", "grade", "direction", and "neutral". By explicitly passing semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS), the colorblind theme will lack the other four mappings. Code that accesses theme.semantic["verdict"] or other missing keys when using COLORBLIND_THEME will raise KeyError.

🛡️ Proposed fixes

Option 1 (recommended): Merge with defaults, override severity only

 COLORBLIND_THEME: Theme = Theme(
     name="colorblind",
     palette=list(COLORBLIND_PALETTE),
-    semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS),
+    semantic={
+        **{k: dict(v) for k, v in SEMANTIC_COLORS.items()},
+        **COLORBLIND_SEMANTIC_COLORS,
+    },
 )

Option 2: Define all semantic mappings in COLORBLIND_SEMANTIC_COLORS

Add the missing keys to COLORBLIND_SEMANTIC_COLORS (lines 171-178):

 COLORBLIND_SEMANTIC_COLORS: dict[str, dict[str, str]] = {
     "severity": {
         "acceptable": "`#0072B2`",
         "moderate": "`#E69F00`",
         "severe": "`#D55E00`",
         "unknown": "`#999999`",
-    }
+    },
+    "verdict": {
+        "deploy": "`#0072B2`",
+        "caution": "`#E69F00`",
+        "do_not_deploy": "`#D55E00`",
+    },
+    "grade": {
+        "A": "`#009E73`",
+        "B": "`#0072B2`",
+        "C": "`#E69F00`",
+        "D": "`#D55E00`",
+    },
+    "direction": {
+        "positive": "`#0072B2`",
+        "negative": "`#E69F00`",
+    },
+    "neutral": {
+        "reference": "`#999999`",
+        "edge": "`#FFFFFF`",
+        "annotation_edge": "`#CCCCCC`",
+        "annotation_face": "`#FFFFFF`",
+    },
 }

Then update line 214:

-    semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS),
+    semantic={k: dict(v) for k, v in COLORBLIND_SEMANTIC_COLORS.items()},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
COLORBLIND_THEME: Theme = Theme(
name="colorblind",
palette=list(COLORBLIND_PALETTE),
semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS),
)
COLORBLIND_THEME: Theme = Theme(
name="colorblind",
palette=list(COLORBLIND_PALETTE),
semantic={
**{k: dict(v) for k, v in SEMANTIC_COLORS.items()},
**COLORBLIND_SEMANTIC_COLORS,
},
)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@trustlens/visualization/style.py` around lines 211 - 215, COLORBLIND_THEME
currently sets semantic=deepcopy(COLORBLIND_SEMANTIC_COLORS) which omits the
default keys from SEMANTIC_COLORS and can cause KeyError; fix by merging the
defaults and the overrides instead of replacing them — for example, create the
theme semantic mapping by starting from a deepcopy of SEMANTIC_COLORS and
updating it with COLORBLIND_SEMANTIC_COLORS so only "severity" is overridden
while "verdict", "grade", "direction", and "neutral" remain present in
COLORBLIND_THEME; alternatively, add all missing semantic keys to
COLORBLIND_SEMANTIC_COLORS so it fully defines every required semantic mapping.

Comment on lines +211 to +215

@llamapreview llamapreview Bot Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 | Confidence: Medium

Speculative: The new COLORBLIND_THEME and its supporting constants (COLORBLIND_PALETTE, COLORBLIND_SEMANTIC_COLORS) are defined and exported in __all__, but no internal plotting function or test references them. The related context shows that all existing plotting modules (e.g., bias_plots.py, fairness.py) call apply_style() with no argument, defaulting to DEFAULT_THEME. The theme is thus dead code at this point. While the PR description states the goal is to “add colorblind-safe visualization theme,” the definition alone does not deliver value to end users; the missing integration creates a false sense of completeness and may lead to confusion if users attempt to use COLORBLIND_THEME without any guidance on how to apply it. Additionally, COLORBLIND_PALETTE and COLORBLIND_SEMANTIC_COLORS are omitted from __all__, making them less discoverable.

Code Suggestion:

def apply_style(theme: Theme | None = None) -> Iterator[Theme]:
    if theme is None:
        # Optionally allow global setting
        theme = _ACTIVE_THEME  # or DEFAULT_THEME
    ...



# ---------------------------------------------------------------------------
# Context manager — scoped rcParams mutations
Expand All @@ -220,29 +224,6 @@ class Theme:
def apply_style(theme: Theme | None = None) -> Iterator[Theme]:
"""
Apply a TrustLens theme inside a ``with`` block, restoring state on exit.

This context manager scopes all ``matplotlib.rcParams`` mutations and base
style sheet changes to the ``with`` block. On exit, the previous rcParams
are restored. This avoids leaking TrustLens styling into the user's
matplotlib session — important when TrustLens runs inside notebooks or
larger ML pipelines.

Parameters
----------
theme : Theme, optional
Theme to apply. Defaults to :data:`DEFAULT_THEME`.

Yields
------
Theme
The active theme, so the caller can read its constants without a
separate import.

Examples
--------
>>> with apply_style() as theme:
... fig, ax = plt.subplots()
... ax.plot([0, 1], [0, 1], color=theme.brand["blue"])
"""
active = theme if theme is not None else DEFAULT_THEME
previous = mpl.rcParams.copy()
Expand Down Expand Up @@ -290,27 +271,6 @@ def styled_figure(
) -> tuple[plt.Figure, Any]:
"""
Create a figure and axes pre-configured with TrustLens styling.

Parameters
----------
figsize : tuple of float, optional
Figure size. Defaults to ``theme.fig_defaults["figsize"]``.
nrows : int
Number of subplot rows.
ncols : int
Number of subplot columns.
theme : Theme, optional
Theme to source defaults from. Defaults to :data:`DEFAULT_THEME`.
grid : bool
Whether to enable the themed grid on the axes.
**subplots_kwargs
Forwarded to :func:`matplotlib.pyplot.subplots`.

Returns
-------
fig : matplotlib.figure.Figure
ax : matplotlib.axes.Axes or array of Axes
Same shape as :func:`matplotlib.pyplot.subplots` returns.
"""
active = theme if theme is not None else DEFAULT_THEME
size = figsize if figsize is not None else active.fig_defaults["figsize"]
Expand All @@ -335,26 +295,6 @@ def styled_figure(
def get_categorical_colors(n: int, theme: Theme | None = None) -> list[str]:
"""
Return ``n`` categorical colors from the active theme palette.

Cycles through the palette when ``n`` exceeds its length, so callers can
request any positive ``n`` without bounds checking.

Parameters
----------
n : int
Number of colors requested. Must be non-negative.
theme : Theme, optional
Theme whose palette to draw from. Defaults to :data:`DEFAULT_THEME`.

Returns
-------
list of str
``n`` hex color strings.

Raises
------
ValueError
When ``n < 0`` or when the theme's palette is empty.
"""
if n < 0:
raise ValueError(f"n must be non-negative, got {n}")
Expand All @@ -363,3 +303,20 @@ def get_categorical_colors(n: int, theme: Theme | None = None) -> list[str]:
if not palette:
raise ValueError("theme palette must be non-empty")
return [palette[i % len(palette)] for i in range(n)]


__all__ = [
"BRAND_COLORS",
"PALETTE",
"SEMANTIC_COLORS",
"TYPOGRAPHY",
"GRID",
"FIG_DEFAULTS",
"SPACING",
"Theme",
"DEFAULT_THEME",
"COLORBLIND_THEME",
"apply_style",
"styled_figure",
"get_categorical_colors",
]