Release v2.6.0 - #6
Merged
Merged
Conversation
Introduce an AppearanceSpace ABC with two backends: - CIELAB (CIE 1976, CIE76 dE*ab or CIEDE2000 dE00 via skimage) - ICtCp (ITU-R BT.2100, BT.2124-0 dE_ITP, HDR-aware) The make_appearance_space(name) factory centralizes creation. Extract the shared srgb_to_linear into colorcast/utils/color_utils.py so both simulation and appearance can import it without duplication. Add **/.DS_Store to .gitignore to catch nested macOS junk. 12 tests cover identical-image zero-dE, monotonicity, PQ/linear helpers, metric names, and input validation.
Add appearance_delta / appearance_delta_name fields to ErrorMap and a preferred_metric() method that centralizes the priority rule (appearance > CIEDE2000 > chroma). get_error_map() accepts an appearance= keyword; compute_dashboard() passes it through. New CLI subcommand colorcast dashboard IMAGE --appearance ictcp. GUI Dashboard dialog gains a dE-metric dropdown that re-triggers computation. Report filenames and titles auto-include the metric label (e.g. dashboard_report_ictcp.png). Fix a GUI test that did not account for the initial _start_computation call from DashboardDialog.__init__, causing it to see 3 pending requests instead of 2.
…rejection - _get_image_dimensions: catch and re-raise Pillow DecompressionBombError so oversized images stop at header read instead of triggering a full decode fallback. Restore Pillow.MAX_IMAGE_PIXELS to its original value after the header check. - Extract _read_image_array for single-frame validation; reject multi-frame stacks (ndim > 3 or ambiguous channel count). - Add tests for DecompressionBombError propagation and multi-frame detection via skimage/imread monkeypatching.
ColorCastConfig.load() now rejects JSON booleans for integer-typed fields (bool is a subclass of int, so isinstance alone was not enough). Floats are still accepted for integer fields because JSON cannot distinguish them.
Add chevron-down.png for combo-box dropdown arrows. Wire MANIFEST.in and pyproject.toml to ship assets/ in sdists. Also pin mypy python_version to 3.10 to match the project's minimum supported Python.
- README test metrics: 378 passing, 1 skipped (up from 361) - README new features: appearance spaces, dashboard CLI, GUI theme - README CLI and Python API examples for appearance + dashboard - CHANGELOG [2.6.0] entries covering all prior commits - Replace dashboard_report.png with metric-labelled ICtCp variant - Update interface screenshots to reflect the new dark theme
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughColorCast 2.6.0 adds CIELAB and ICtCp appearance metrics to analysis, error maps, dashboards, CLI, and GUI workflows. It also hardens image loading, centralizes sRGB conversion, updates the GUI theme, packages GUI assets, and updates release metadata. ChangesAppearance metrics and analysis
CLI and GUI
Supporting changes
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant cmd_dashboard
participant load_image
participant compute_dashboard
participant report_generator
CLI->>cmd_dashboard: select image and appearance space
cmd_dashboard->>load_image: load input image
cmd_dashboard->>compute_dashboard: calculate deficiency metrics
compute_dashboard-->>cmd_dashboard: return dashboard result
cmd_dashboard->>report_generator: write metric-labeled report
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
colorcast/processing/image_loader.py (1)
258-263: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve
InvalidImageFormatErrorfrom_read_image_array.
_read_image_arrayraisesInvalidImageFormatErrorfor unsupported frame stacks and channel layouts. The generic handler converts that expected validation result intoImageLoadError. This conflicts with the documented public exception contract.Proposed fix
try: img = img_as_float(_read_image_array(path)) + except InvalidImageFormatError: + raise except OSError as e: raise ImageLoadError(f"Failed to read image file: {e}") from e🤖 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 `@colorcast/processing/image_loader.py` around lines 258 - 263, Update the exception handling around _read_image_array in the image-loading flow to let InvalidImageFormatError propagate unchanged before the generic Exception handler runs. Preserve the existing ImageLoadError wrapping for other unexpected errors and the OSError handling.colorcast/analysis/error_map.py (1)
270-274: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale inline comment about NaN.
Line 274 now assigns
Nonewhencompute_dE00is false. The comment on lines 272-273 still states that the result contains NaN values. The class docstring at line 88 was updated toNone. Align this comment.📝 Proposed comment fix
# -- CIEDE2000 chromaticity error ----------------------------------------- # Compute dE00 with L* pinned to 50 to isolate chromaticity contribution. - # By default this optional output is disabled, so the result contains - # NaN values instead of a computed metric. + # By default this optional output is disabled, so the field is left as + # ``None`` instead of holding a computed metric. chroma_error_dE00 = _compute_chroma_error_dE00(orig_lab, sim_lab) if compute_dE00 else None🤖 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 `@colorcast/analysis/error_map.py` around lines 270 - 274, Update the inline comment above chroma_error_dE00 to state that the optional output is disabled by assigning None when compute_dE00 is false, replacing the stale reference to NaN; leave the _compute_chroma_error_dE00 assignment unchanged.colorcast/gui.py (1)
1010-1022: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale chroma-loss naming.
Line 1019 now renders
em.preferred_metric(), which can be the appearance ΔE, CIEDE2000, or the Euclidean chroma error. The comment on lines 1010-1011 still describes thechroma_errorfield.The placeholder cell titles in
_build_uihave the same problem. They read"Chroma Loss (P)"until line 1022 replaces them with_heatmap_title. Until the first result arrives, the dialog shows a metric name the user did not select.📝 Proposed fix
- # Chroma-loss heatmaps — use the chroma_error field rendered as - # a grayscale hot image + # Metric heatmaps — render the preferred per-pixel metric as a + # grayscale hot image and retitle the cell with the metric label. for key, deficiency in [Also make the placeholder titles metric-neutral in
_build_ui:- (2, 0, "heatmap_protanopia", "Chroma Loss (P)"), - (2, 1, "heatmap_deuteranopia", "Chroma Loss (D)"), - (2, 2, "heatmap_tritanopia", "Chroma Loss (T)"), + (2, 0, "heatmap_protanopia", "ΔE (P)"), + (2, 1, "heatmap_deuteranopia", "ΔE (D)"), + (2, 2, "heatmap_tritanopia", "ΔE (T)"),🤖 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 `@colorcast/gui.py` around lines 1010 - 1022, Remove the stale chroma-loss wording from the heatmap comment near the loop using em.preferred_metric(), describing it instead as the selected error metric. In _build_ui, replace the placeholder “Chroma Loss” titles for the protanopia, deuteranopia, and tritanopia heatmaps with metric-neutral titles that do not imply a specific metric.
🧹 Nitpick comments (3)
colorcast/analysis/dashboard.py (1)
266-272: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the
chroma_errorparameter to match the generalized metric.The docstring on line 267 now says "metric heatmap". Callers at line 240 pass
em.preferred_metric(), which can be the appearance ΔE, CIEDE2000, or the Euclidean chroma error. The parameter namechroma_errorno longer describes the value.♻️ Proposed rename
-def _show_heatmap(ax, chroma_error: np.ndarray) -> None: +def _show_heatmap(ax, metric: np.ndarray) -> None: """Render a metric heatmap on a Matplotlib axis.""" - vmax = float(chroma_error.max()) + vmax = float(metric.max()) if vmax < 1e-6: - ax.imshow(np.zeros_like(chroma_error), cmap="hot", vmin=0, vmax=1) + ax.imshow(np.zeros_like(metric), cmap="hot", vmin=0, vmax=1) else: - ax.imshow(chroma_error / vmax, cmap="hot", vmin=0, vmax=1) + ax.imshow(metric / vmax, cmap="hot", vmin=0, vmax=1) ax.axis("off")🤖 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 `@colorcast/analysis/dashboard.py` around lines 266 - 272, Rename the _show_heatmap parameter from chroma_error to a generalized metric name, and update all references within the function, including the vmax calculation, zero-array fallback, and normalization expression; preserve the existing rendering behavior for any preferred metric.tests/test_appearance.py (1)
86-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
make_appearance_spaceand the"linear"transfer function.
make_appearance_spaceis a new public export. The CLI and the GUI both depend on it. No test covers it, including theValueErrorbranch for an unknown name.The
"linear"transfer function path is also untested. A test there would catch the in-place mutation flagged incolorcast/analysis/appearance.py.💚 Proposed additional tests
def test_make_appearance_space_returns_expected_types() -> None: from colorcast.analysis.appearance import make_appearance_space assert isinstance(make_appearance_space("cielab"), CIELABSpace) assert isinstance(make_appearance_space("ICtCp"), ICtCpSpace) def test_make_appearance_space_rejects_unknown_name() -> None: from colorcast.analysis.appearance import make_appearance_space with pytest.raises(ValueError, match="unsupported appearance space"): make_appearance_space("oklab") def test_ictcp_linear_transfer_does_not_mutate_source_array() -> None: rgb = np.random.rand(4, 4, 3).astype(np.float64) original = rgb.copy() ICtCpSpace(transfer_function="linear").from_rgb(rgb) np.testing.assert_array_equal(rgb, original)🤖 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 `@tests/test_appearance.py` around lines 86 - 111, Add tests in tests/test_appearance.py covering make_appearance_space: verify “cielab” and “ICtCp” return CIELABSpace and ICtCpSpace, and unknown names raise ValueError matching “unsupported appearance space”. Also test ICtCpSpace(transfer_function="linear").from_rgb does not mutate the source RGB array by comparing it with a copy after conversion.tests/test_gui.py (1)
239-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the stale-completion assertion and use
monkeypatchfor the class attribute.Two points.
First, lines 239-244 patch
DashboardDialog._start_computationon the class directly. Thefinallyblock restores it, so the test is correct. The test at line 210 already usesmonkeypatch.setattrfor the same attribute. Usemonkeypatchhere too, and callmonkeypatch.undo()after construction.Second, the assertion on lines 255-256 only proves that
NonestaysNone. The test name states that a stale completion does not mutate the current result. Seeddialog._resultwith a sentinel first, then assert the sentinel survives the stale callback.💚 Proposed test changes
- # Suppress the initial _start_computation call from __init__ so the - # test controls exactly how many requests are queued. - _original_start = DashboardDialog._start_computation - DashboardDialog._start_computation = lambda self: None - try: - dialog = DashboardDialog(np.random.rand(8, 8, 3).astype(np.float32)) - finally: - DashboardDialog._start_computation = _original_start + # Suppress the initial _start_computation call from __init__ so the + # test controls exactly how many requests are queued. + monkeypatch.setattr(DashboardDialog, "_start_computation", lambda self: None) + dialog = DashboardDialog(np.random.rand(8, 8, 3).astype(np.float32)) + monkeypatch.undo() + monkeypatch.setattr(gui_module, "_run_in_thread", fake_run) monkeypatch.setattr(dialog, "_populate_results", lambda: None) + + # Seed a current result so a stale completion has something to clobber. + sentinel = object() + dialog._result = sentinel dialog._start_computation() dialog._start_computation() assert len(pending) == 2 pending[0][0]() pending[0][1]() - assert dialog._result is None + assert dialog._result is sentinel assert dialog._error is NoneNote that
monkeypatch.undo()reverts every patch, so reapply the_run_in_threadpatch afterwards as shown.🤖 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 `@tests/test_gui.py` around lines 239 - 256, Update the test around DashboardDialog._start_computation to use monkeypatch.setattr instead of direct class assignment, call monkeypatch.undo() after constructing the dialog, and reapply the existing _run_in_thread patch afterward if needed. Before invoking the stale completion callbacks, assign dialog._result a sentinel value and assert that the sentinel remains unchanged, while preserving the existing _error assertion.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@colorcast/analysis/appearance.py`:
- Around line 249-254: Update the linear branch of _rgb_to_ictcp to create an
independent float64 copy of rgb before the in-place peak_luminance scaling,
while preserving the existing srgb branch and conversion behavior.
In `@colorcast/gui.py`:
- Around line 60-61: Update the QSS template usages of `@CHEVRON_DOWN`@, including
the references near the drop-down styling and related rules, so the substituted
filesystem path is enclosed in quotes within url(). Keep _CHEVRON_PATH unchanged
and ensure every occurrence produces valid QSS when the path contains spaces.
- Around line 955-961: Update the appearance combo-box setup and selection flow
around _appearance_combo so each item stores its appearance-space id as item
data, then use currentData() instead of matching display text in the space_name
assignment. Preserve the existing CIELAB fallback when no combo box data is
available, and pass the selected id to make_appearance_space.
In `@colorcast/processing/image_loader.py`:
- Around line 93-100: Update the exception handling in _get_image_dimensions in
image_loader.py so DecompressionBombWarning is handled before the generic
fallback and does not reach the broad Exception branch. Mirror the existing
DecompressionBombError path by re-raising or converting DecompressionBombWarning
to ValidationError, while leaving the OSError/ValueError pass-through and the
logger.warning fallback for unrelated exceptions unchanged.
---
Outside diff comments:
In `@colorcast/analysis/error_map.py`:
- Around line 270-274: Update the inline comment above chroma_error_dE00 to
state that the optional output is disabled by assigning None when compute_dE00
is false, replacing the stale reference to NaN; leave the
_compute_chroma_error_dE00 assignment unchanged.
In `@colorcast/gui.py`:
- Around line 1010-1022: Remove the stale chroma-loss wording from the heatmap
comment near the loop using em.preferred_metric(), describing it instead as the
selected error metric. In _build_ui, replace the placeholder “Chroma Loss”
titles for the protanopia, deuteranopia, and tritanopia heatmaps with
metric-neutral titles that do not imply a specific metric.
In `@colorcast/processing/image_loader.py`:
- Around line 258-263: Update the exception handling around _read_image_array in
the image-loading flow to let InvalidImageFormatError propagate unchanged before
the generic Exception handler runs. Preserve the existing ImageLoadError
wrapping for other unexpected errors and the OSError handling.
---
Nitpick comments:
In `@colorcast/analysis/dashboard.py`:
- Around line 266-272: Rename the _show_heatmap parameter from chroma_error to a
generalized metric name, and update all references within the function,
including the vmax calculation, zero-array fallback, and normalization
expression; preserve the existing rendering behavior for any preferred metric.
In `@tests/test_appearance.py`:
- Around line 86-111: Add tests in tests/test_appearance.py covering
make_appearance_space: verify “cielab” and “ICtCp” return CIELABSpace and
ICtCpSpace, and unknown names raise ValueError matching “unsupported appearance
space”. Also test ICtCpSpace(transfer_function="linear").from_rgb does not
mutate the source RGB array by comparing it with a copy after conversion.
In `@tests/test_gui.py`:
- Around line 239-256: Update the test around DashboardDialog._start_computation
to use monkeypatch.setattr instead of direct class assignment, call
monkeypatch.undo() after constructing the dialog, and reapply the existing
_run_in_thread patch afterward if needed. Before invoking the stale completion
callbacks, assign dialog._result a sentinel value and assert that the sentinel
remains unchanged, while preserving the existing _error assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cf61ea8-458f-4755-91f8-fb6b2f0d0e75
⛔ Files ignored due to path filters (7)
colorcast/assets/chevron-down.pngis excluded by!**/*.pngimgs/CVD-Accessibility-Dashboard.pngis excluded by!**/*.pngimgs/Compare-Transfer-Methods.pngis excluded by!**/*.pngimgs/dashboard_report.pngis excluded by!**/*.pngimgs/dashboard_report_ICtCp.pngis excluded by!**/*.pngimgs/interface.pngis excluded by!**/*.pngimgs/interface_2.pngis excluded by!**/*.png
📒 Files selected for processing (23)
.gitignore.zenodo.jsonCHANGELOG.mdCITATION.cffMANIFEST.inREADME.mdcolorcast/__main__.pycolorcast/_version.pycolorcast/analysis/__init__.pycolorcast/analysis/appearance.pycolorcast/analysis/dashboard.pycolorcast/analysis/error_map.pycolorcast/gui.pycolorcast/processing/gpu_transfer.pycolorcast/processing/image_loader.pycolorcast/processing/simulation.pycolorcast/utils/color_utils.pycolorcast/utils/config.pydocs/index.rstpyproject.tomltests/test_appearance.pytests/test_gui.pytests/test_image_loading.py
- appearance.py: make a full copy before in-place peak-luminance scaling in the linear branch so the caller's array is not mutated. - gui.py: wrap chevron asset path in url() quotes so QSS survives paths with spaces; use combo-box itemData/currentData instead of text matching for appearance-space selection. - image_loader.py: catch DecompressionBombWarning and convert it to DecompressionBombError before it reaches the generic Exception fallback; preserve cause chain with from e.
- dashboard.py: rename _show_heatmap parameter from chroma_error to metric to reflect that it accepts any preferred metric. - test_gui.py: use monkeypatch.setattr instead of direct class assignment for _start_computation; add sentinel check to verify stale completions do not mutate the current result. - test_appearance.py: add make_appearance_space type/error tests and ICtCpSpace linear-transfer non-mutation test.
- image_loader.py: let InvalidImageFormatError propagate unchanged before the generic Exception handler wraps other errors. - error_map.py: update stale NaN comment to reflect that the optional chroma_error_dE00 field is left as None, not populated with NaN. - gui.py: rename Chroma Loss heatmap titles to metric-neutral Error Metric labels; update heatmap comment to reference preferred_metric() instead of chroma_error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's new
Added
make_appearance_space(name)factory,get_error_map(appearance=), andcompute_dashboard(appearance=).colorcast dashboardCLI subcommand — generate full-resolution CVD accessibility reports with--appearance {cielab,ictcp}.dashboard_report_ictcp.png).ErrorMap.preferred_metric()— centralizes the priority rule (appearance > CIEDE2000 > chroma) so downstream code stays ignorant of which metric was computed.srgb_to_linearutility incolorcast/utils/color_utils.py— eliminates the duplicate fromsimulation.py.Fixed
DecompressionBombErrornow re-raises instead of falling through to a full decode. Multi-frame stacks rejected before allocation.ColorCastConfig.load()no longer silently accepts JSON booleans for integer-typed fields._start_computationcall fromDashboardDialog.__init__.Changed
CIELABSpacedefault metric is now CIEDE2000 (was CIE76)._summarizesimplified — delegates toErrorMap.preferred_metric()instead of encoding the priority chain inline.gpu_transfer.pymodule docstring simplified.Tests
Summary by CodeRabbit
New Features
Bug Fixes
Documentation