Release v2.5.0 - #1
Conversation
Set release date to 2026-08-02 in CHANGELOG and CITATION.cff. Update _version.py docstring to reflect dynamic version in pyproject.toml.
Added: - CVD accessibility dashboard (colorcast/analysis/dashboard.py) - Headless GUI smoke tests (tests/test_gui.py) - --verbose flag on CLI with opt-in tracebacks - Path-containment regression tests (tests/test_validators_enhanced.py) - Ruff configuration in pyproject.toml - Daltonization efficacy script (scripts/) Changed: - CLI builds kwargs from method.parameters, preventing signature clashes - Version centralized in _version.py; pyproject.toml uses dynamic version - GPU contract: gpu_histogram_matching CPU-only, CuPy warning removed - extract module-level constants (_EPSILON, _LAB_L_BOUNDS, _LAB_AB_BOUNDS) - StyleTransferApp derives settings from ColorCastConfig - apply_daltonization refactored with _compute_chromaticity_weight helper - validate_and_resize_images return annotation tightened - CLI intensity now validates bounds; error exit codes remapped - batch.py catches FileNotFoundError and OSError - __init__.py re-exports CVD and Daltonization symbols Fixed: - daltonize(intensity=0) returns original image instead of simulated - selective_color_transfer uses smoothstep masks (no hard edges) - save_image re-raises with from e to preserve cause chain - show_image copies QImage before QPixmap to prevent GC crashes - StyleTransferCache is now thread-safe with threading.Lock - validate_file_path rejects sibling-prefix bypass - _compute_hash uses downsampled fingerprint instead of full image bytes - MethodComparison.find_best_method infers metric direction - ColorCastConfig.load() validates types; enable_parallel wired - B904 violations fixed: raise ... from e Removed: - Dead GPU round-trip in gpu_histogram_matching - ColorCastConfig.cache_size unused field - Module-level CuPy import warning - requirements.txt, requirements-dev.txt - Legacy colorcast.py
Add GitHub Actions CI workflow (lint, mypy, test matrix across Python 3.10-3.13). Add PyPI publish workflow via Trusted Publisher (OIDC). Add Dependabot configuration for pip and GitHub Actions. Add issue/PR templates, CODE_OF_CONDUCT, CONTRIBUTING, and SECURITY.
Update test metrics to match current suite (75% coverage, 359 passed, 1 skipped). Add CVD dashboard, method comparison, and dashboard report screenshots. Rewrite Use Cases section to comply with project writing standards. Add tests/ to lint commands in Contributing section.
📝 WalkthroughWalkthroughColorCast 2.5.0 adds CVD accessibility analysis, Lab-space Daltonization, dashboard and comparison dialogs, centralized configuration and versioning, improved CLI validation, cache changes, image validation, CI automation, documentation, and expanded tests. ChangesColorCast 2.5.0 release
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StyleTransferApp
participant DashboardDialog
participant compute_dashboard
participant generate_dashboard_report
User->>StyleTransferApp: Select Dashboard
StyleTransferApp->>DashboardDialog: Open with content image
DashboardDialog->>compute_dashboard: Run asynchronous analysis
compute_dashboard-->>DashboardDialog: Return DashboardResult
User->>DashboardDialog: Export report
DashboardDialog->>generate_dashboard_report: Save report
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Bump mypy target from 3.10 to 3.12 to match CI interpreter. Fix pre-existing type errors in comparison, validators, image_loader, dashboard, and daltonization modules. Suppress PyQt5 stub errors in gui.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
colorcast/analysis/comparison.py (1)
252-262: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the identity exclusion in
rank_methods.The new filter drops every method whose
color_distanceis close to zero, for all values ofprimary_metric.generate_comparison_reportLines 309-318 callsrank_methodsfor each metric, so such a method appears in the metrics table but in none of the ranking sections. Onlyfind_best_methoddocuments this rule at Lines 330-334. Add the same statement to therank_methodsdocstring, becauserank_methodsis a public method.🤖 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/comparison.py` around lines 252 - 262, Update the public rank_methods docstring to document that methods with color_distance close to zero are excluded from rankings for every primary_metric, matching the existing behavior and the explanation in find_best_method. Keep the filtering logic unchanged.colorcast/utils/validators_enhanced.py (1)
224-242: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject unsupported formats in the Pillow fallback path too.
The
imghdrbranch (lines 255-259) rejects a file whenexpected_extis empty, treating it as an unsupported type. The Pillow fallback branch (used whenimghdris unavailable, i.e. Python 3.13+) does not perform this check: it calls_check_extension_matches(expected_ext, path_ext, fmt or "unknown")and returns unconditionally._check_extension_matchesreturns silently whendetected_extis empty, so a file whose PIL-detected format isn't in_PIL_FORMAT_TO_EXT(for example ICO, PSD, or MPO) passes validation as long as its extension is one of the allowed ones. This bypasses the file-type-spoofing protection this function documents, specifically on the Python version this fallback targets.Add the same unsupported-type check to the Pillow branch.
🛡️ Proposed fix to reject unmapped formats in the Pillow branch
path_ext = path_obj.suffix.lower() expected_ext = _PIL_FORMAT_TO_EXT.get(fmt or "") _check_extension_matches(expected_ext, path_ext, fmt or "unknown") + if not expected_ext: + raise ValidationError( + f"Unsupported image type: {fmt or 'unknown'}. " + f"Supported types: JPEG, PNG, BMP, TIFF, GIF, WebP" + ) return🤖 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/utils/validators_enhanced.py` around lines 224 - 242, In the Pillow fallback branch, update the logic after deriving expected_ext from _PIL_FORMAT_TO_EXT to reject formats whose mapping is missing, matching the imghdr branch’s unsupported-type behavior. Perform this check before _check_extension_matches and return only for supported mapped formats.
🧹 Nitpick comments (12)
colorcast/analysis/dashboard.py (1)
23-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExport the deficiency constants publicly.
_DEFICIENCIESand_DEFICIENCY_LABELSuse the private naming convention, butcolorcast/gui.pyLines 28-33 imports both across module boundaries. Rename them toDEFICIENCIESandDEFICIENCY_LABELS, and list them in the module docstring "Public API" section. This makes the dependency explicit and prevents a later rename from breaking the GUI silently.🤖 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 23 - 28, Rename _DEFICIENCIES and _DEFICIENCY_LABELS to the public names DEFICIENCIES and DEFICIENCY_LABELS, update all references including the imports in colorcast/gui.py, and add both names to the module docstring’s Public API section.colorcast/analysis/visualization.py (1)
188-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new histogram rows.
show_histogramsdefaults to False invisualize_method_comparison, and the calls intests/test_visualization.pyLines 129-148 never set it to True. The new row-offset arithmetic at Line 193 and the per-method axes indexing at Lines 202-208 are therefore not exercised. Add a test that callsvisualize_method_comparison(images, reference, show_histograms=True)for both values ofshow_difference.🤖 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/visualization.py` around lines 188 - 208, Add coverage in tests/test_visualization.py by calling visualize_method_comparison with show_histograms=True for both show_difference=True and show_difference=False. Ensure the assertions exercise the histogram row offset and per-method axes indexing implemented in the show_histograms block.colorcast/analysis/comparison.py (1)
130-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReconcile the metric value types.
_make_metricsreturnsdict[str, float | str]because of the_errorkey.compare_methodsis annotated-> dict[str, dict[str, float]]and stores those dicts inresults, which is declared asdict[str, dict[str, float]]at Line 201. A static type checker reports an incompatible assignment here.Widen the
compare_methodsreturn type and theresultsdeclaration todict[str, dict[str, float | str]], and widen therank_methods,generate_comparison_report, andfind_best_methodparameters in the same way.Also applies to: 205-218
🤖 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/comparison.py` around lines 130 - 136, Reconcile metric container annotations by widening compare_methods’ return type, its results declaration, and the parameter types of rank_methods, generate_comparison_report, and find_best_method to dict[str, dict[str, float | str]], matching _make_metrics and allowing the _error string value.colorcast/analysis/daltonization.py (1)
108-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated section header.
Lines 108-112 contain two stacked banner comments.
Convenience end-to-end pipelineis repeated at lines 198-200 abovedaltonize, where it belongs. Keep onlyCore functionhere.♻️ Proposed cleanup
-# --------------------------------------------------------------------------- -# Convenience end-to-end pipeline -# --------------------------------------------------------------------------- # Core function # ---------------------------------------------------------------------------🤖 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/daltonization.py` around lines 108 - 112, Remove the duplicated “Convenience end-to-end pipeline” banner from the section around “Core function” in daltonization.py, leaving only the “Core function” header there; retain the separate convenience-pipeline header above daltonize.colorcast/gui.py (2)
268-297: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstrain the
image_typerole.
image_typeis a plainstr. Line 288 treats every value other than"content"as the style role, so a typo assigns the image toself.style_imagewithout any error. Annotate the parameter asLiteral["content", "style"]so a type checker rejects other values.♻️ Proposed change
+from typing import Literal + def _load_image_file( self, path: str, - image_type: str, + image_type: Literal["content", "style"], preview_label: QLabel, ) -> 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/gui.py` around lines 268 - 297, Constrain the _load_image_file image_type parameter to Literal["content", "style"] instead of plain str, adding the necessary typing import. Preserve the existing content/style branching while enabling type checkers to reject invalid role values.
747-758: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the cached transfer and record the failure reason.
Two points:
- Line 752 calls
instance.transferdirectly.apply_style_transferLine 416 callsregistry.transfer_cached. The comparison dialog therefore recomputes every method on each open and does not populate the shared cache. Useregistry.transfer_cached(method_id, self._content_image, self._style_image)for consistent behavior.- Line 756 logs at debug level without the exception. A user sees only a reduced count in the status text at Line 776 and has no way to find the cause. Use
logger.exceptionand keep the method name.♻️ Proposed change
def _run() -> None: for method_id in self._label_widgets: try: - instance = registry.get_method(method_id) - self._results[method_id] = instance.transfer( - self._content_image, self._style_image + self._results[method_id] = registry.transfer_cached( + method_id, self._content_image, self._style_image ) - except Exception: # noqa: BLE001 — skip unsupported methods - logger.debug("Skipped method %s in comparison", method_id) + except Exception: # noqa: BLE001 — skip unsupported methods + logger.exception("Skipped method %s in comparison", method_id)🤖 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 747 - 758, Update _start_computation’s _run loop to call registry.transfer_cached(method_id, self._content_image, self._style_image) instead of instance.transfer, preserving shared-cache behavior. In the exception handler, replace the debug log with logger.exception while retaining the method identifier so the failure reason and affected method are recorded.pyproject.toml (1)
47-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating overlapping lint/format tools.
Ruff's lint selection now includes
"I"(isort rules) at line 106, butisortandblackremain separate dev dependencies at lines 48-49, and the PR checklist still requires runningisort --check-onlyseparately. Running three tools for overlapping concerns (import sorting, formatting) can produce conflicting fixes over time.Consider standardizing on
ruff formatand Ruff'sIrules, and droppingblack/isortonce CI is updated accordingly.Also applies to: 105-106
🤖 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 `@pyproject.toml` around lines 47 - 57, Consolidate the formatting and import-sorting tooling by removing the black and isort development dependencies, standardizing on ruff format and the existing Ruff I lint rules in the Ruff configuration, and updating CI and the PR checklist to stop invoking isort or black separately..github/workflows/ci.yml (1)
16-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin all GitHub Actions to immutable commits.
All action references in
.github/workflows/ci.ymland.github/workflows/publish.ymluse mutable tags or branches. Pin every action to a full commit SHA. Keep the GitHub Actions Dependabot configuration so the pins remain current.🤖 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 @.github/workflows/ci.yml around lines 16 - 19, Update every action reference in the workflows’ uses entries, including actions/checkout, actions/setup-python, and all actions in ci.yml and publish.yml, to a full immutable commit SHA instead of a tag or branch. Preserve the existing GitHub Actions Dependabot configuration so these pinned revisions continue receiving updates.colorcast/processing/gpu_transfer.py (2)
37-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer CuPy's built-in availability check.
is_gpu_available()reimplementscupy.is_available(). That built-in function performs the driver query and device-count check this code hand-rolls. Use it instead of a custom implementation to reduce maintenance and picking up CuPy's own edge-case handling (for example, driver-initialization failures beyondCUDARuntimeError).♻️ Proposed refactor
def is_gpu_available() -> bool: """Check whether CuPy is installed and a usable CUDA device is present.""" if not HAS_CUPY: return False - try: - return cp.cuda.runtime.getDeviceCount() > 0 - except cp.cuda.runtime.CUDARuntimeError: - return False + return cp.is_available()🤖 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/gpu_transfer.py` around lines 37 - 44, Update is_gpu_available() to use CuPy’s built-in cp.is_available() result instead of manually checking HAS_CUPY and querying getDeviceCount(), while preserving the false result when CuPy is unavailable.
19-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCross-module import of underscore-prefixed "private" symbols.
This module imports
_EPSILON,_LAB_AB_BOUNDS,_LAB_L_BOUNDS, and_meanstd_transferfromcolorcast.processing.transfer_methods. The leading underscore in Python signals module-private, but these symbols are now part of a cross-module contract betweengpu_transfer.pyandtransfer_methods.py. A future contributor could reasonably rename or inline these thinking they are private totransfer_methods.py, silently breakinggpu_transfer.py.Rename these without the leading underscore (or move them into a small shared internal module) to make the cross-module dependency explicit.
🤖 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/gpu_transfer.py` around lines 19 - 25, Make the cross-module dependencies imported by gpu_transfer explicit by renaming _EPSILON, _LAB_AB_BOUNDS, _LAB_L_BOUNDS, and _meanstd_transfer in transfer_methods and updating every reference, including gpu_transfer, to their non-underscore names; keep validate_and_resize_images unchanged.colorcast/processing/transfer_methods.py (1)
16-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring's dtype claim does not hold for the Lab call site.
The docstring for
_meanstd_transferstates that both arrays "must be three-channel float32 images."color_transfer_lab(Line 225) and the Lab fallback incolorcast/processing/gpu_transfer.pycall this function withfloat64Lab arrays fromcolor.rgb2lab. NumPy does not enforce the claim at runtime, so nothing breaks today, but the inaccurate docstring could mislead a future contributor into adding a float32 assertion that would break the Lab callers.📝 Proposed docstring fix
""" Per-channel mean and standard deviation transfer. The caller is responsible for validating and resizing inputs before - calling this function. Both arrays must be three-channel float32 - images with identical spatial dimensions. + calling this function. Both arrays must be three-channel arrays + (float32 RGB or float64 Lab) with identical spatial dimensions.🤖 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/transfer_methods.py` around lines 16 - 42, Update the _meanstd_transfer docstring to remove the incorrect requirement that inputs be float32, and describe them as three-channel floating-point images while preserving the existing shape and spatial-dimension requirements. Do not add runtime validation or alter the transfer logic or Lab call sites.colorcast/processing/cache.py (1)
91-109: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFull-image MD5 hashing on every cache operation adds CPU and memory cost for large images.
_compute_hashnow hashes the complete contiguous byte buffer of the image, instead of a sampled subset.image_loader.pyallows images up to 50 megapixels. Hashing a large float32 array with MD5 costs measurable CPU time and a temporary byte-buffer copy fromtobytes().StyleTransferCache.generate_key()/get()/set()run this hash on the content image and, when present, the style image on every call. If this cache backs interactive operations (for example, a GUI slider that triggers repeated transfers), the added latency accumulates per interaction.Consider one of these approaches if this proves to be a bottleneck:
- Use a faster non-cryptographic hash (for example
xxhashorhashlib.blake2bwith a small digest size) since collision resistance against adversarial input is not a requirement here.- Keep a fast structural fingerprint (shape, dtype, strides) plus a cheap running checksum over strided samples, and fall back to full-content hashing only when the structural fingerprint matches, to avoid a full false-positive cache hit while limiting the common-case cost.
This is a deliberate correctness fix over the old sampled hash, so the cost may be an accepted trade-off. Confirm this is acceptable for the largest images the caching path is expected to process.
🤖 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/cache.py` around lines 91 - 109, Validate the performance impact of the full-image hashing implemented by _compute_hash for the largest images supported by the cache, including repeated generate_key, get, and set calls with content and style images. If the cost is unacceptable, replace MD5 with a faster suitable hash or implement a structural fingerprint with sampled checks and full-content fallback while preserving collision-safe cache correctness; otherwise retain the current full-content behavior and document the accepted trade-off.
🤖 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 `@CHANGELOG.md`:
- Line 44: Update the 2.5.0 test metrics in CHANGELOG.md from 359 passed to the
authoritative 361 passed while retaining 1 skipped and 75% coverage, and ensure
the corresponding README metrics use the same counts.
In `@colorcast/__init__.py`:
- Around line 26-38: Remove the eager analysis imports from the package
initializer, especially MethodComparison, daltonize, and ErrorMap/get_error_map,
so importing colorcast does not initialize colorcast.analysis or require
optional matplotlib. Preserve access to these analysis APIs through lazy imports
or their existing submodules without promoting matplotlib to a base dependency.
In `@colorcast/analysis/daltonization.py`:
- Around line 15-29: Update the Lab-space correction documentation in
apply_daltonization and its repeated description to state that base_lab[:, :, 0]
is preserved, retaining the base image’s L* value rather than restoring the
original image’s lightness. Also revise the ErrorMap.orig_l_star docstring to
remove the claim that apply_daltonization uses it to restore original luminance.
In `@colorcast/analysis/dashboard.py`:
- Around line 54-61: Normalize image_array to float32 in the [0, 1] range inside
compute_dashboard before assigning it to DashboardResult.original, using the
same normalization behavior as ColorBlindSimulator.transform_color_space. Keep
the normalized original separate from the raw input used for simulation if
needed, and preserve the existing result structure and parallel processing.
In `@colorcast/analysis/error_map.py`:
- Line 93: Update the exported ErrorMap NamedTuple definition to keep its
existing positional fields and indexes unchanged; do not add chroma_error_dE00
as a required tuple field. Store the optional dE00 metric outside the positional
tuple contract while preserving compute_dE00=True behavior for the production
reader.
In `@colorcast/gui.py`:
- Around line 71-85: Update the nested _worker function in _run_in_thread so
signals.finished.emit() executes in a finally block around target(), ensuring
completion callbacks run even when the target raises while preserving normal
exception propagation.
In `@docs/index.rst`:
- Around line 41-44: Update the docs/index.rst toctree so the existing
Color-Vision-Background documentation is included using a Sphinx-supported .rst
page, or remove the orphaned docs/wiki/Color-Vision-Background.md file if it
should not be published; do not leave the Markdown page omitted from the
documentation build.
In `@README.md`:
- Line 20: Update the README test-count references at both locations, changing
359 to 361 while preserving the existing “1 skipped” text at the second
location.
In `@SECURITY.md`:
- Around line 20-28: Update the Supported Versions table in SECURITY.md to mark
2.5.x as supported and change the unsupported version entry from < 2.4 to < 2.5,
keeping the existing formatting and support markers.
In `@tests/test_visualization.py`:
- Around line 165-167: Update test_custom_title to capture the result of
visualize_color_channels(image, title="Custom Title") and assert it is an
instance of Figure, matching the return-value checks in test_returns_figure and
test_custom_figsize.
---
Outside diff comments:
In `@colorcast/analysis/comparison.py`:
- Around line 252-262: Update the public rank_methods docstring to document that
methods with color_distance close to zero are excluded from rankings for every
primary_metric, matching the existing behavior and the explanation in
find_best_method. Keep the filtering logic unchanged.
In `@colorcast/utils/validators_enhanced.py`:
- Around line 224-242: In the Pillow fallback branch, update the logic after
deriving expected_ext from _PIL_FORMAT_TO_EXT to reject formats whose mapping is
missing, matching the imghdr branch’s unsupported-type behavior. Perform this
check before _check_extension_matches and return only for supported mapped
formats.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 16-19: Update every action reference in the workflows’ uses
entries, including actions/checkout, actions/setup-python, and all actions in
ci.yml and publish.yml, to a full immutable commit SHA instead of a tag or
branch. Preserve the existing GitHub Actions Dependabot configuration so these
pinned revisions continue receiving updates.
In `@colorcast/analysis/comparison.py`:
- Around line 130-136: Reconcile metric container annotations by widening
compare_methods’ return type, its results declaration, and the parameter types
of rank_methods, generate_comparison_report, and find_best_method to dict[str,
dict[str, float | str]], matching _make_metrics and allowing the _error string
value.
In `@colorcast/analysis/daltonization.py`:
- Around line 108-112: Remove the duplicated “Convenience end-to-end pipeline”
banner from the section around “Core function” in daltonization.py, leaving only
the “Core function” header there; retain the separate convenience-pipeline
header above daltonize.
In `@colorcast/analysis/dashboard.py`:
- Around line 23-28: Rename _DEFICIENCIES and _DEFICIENCY_LABELS to the public
names DEFICIENCIES and DEFICIENCY_LABELS, update all references including the
imports in colorcast/gui.py, and add both names to the module docstring’s Public
API section.
In `@colorcast/analysis/visualization.py`:
- Around line 188-208: Add coverage in tests/test_visualization.py by calling
visualize_method_comparison with show_histograms=True for both
show_difference=True and show_difference=False. Ensure the assertions exercise
the histogram row offset and per-method axes indexing implemented in the
show_histograms block.
In `@colorcast/gui.py`:
- Around line 268-297: Constrain the _load_image_file image_type parameter to
Literal["content", "style"] instead of plain str, adding the necessary typing
import. Preserve the existing content/style branching while enabling type
checkers to reject invalid role values.
- Around line 747-758: Update _start_computation’s _run loop to call
registry.transfer_cached(method_id, self._content_image, self._style_image)
instead of instance.transfer, preserving shared-cache behavior. In the exception
handler, replace the debug log with logger.exception while retaining the method
identifier so the failure reason and affected method are recorded.
In `@colorcast/processing/cache.py`:
- Around line 91-109: Validate the performance impact of the full-image hashing
implemented by _compute_hash for the largest images supported by the cache,
including repeated generate_key, get, and set calls with content and style
images. If the cost is unacceptable, replace MD5 with a faster suitable hash or
implement a structural fingerprint with sampled checks and full-content fallback
while preserving collision-safe cache correctness; otherwise retain the current
full-content behavior and document the accepted trade-off.
In `@colorcast/processing/gpu_transfer.py`:
- Around line 37-44: Update is_gpu_available() to use CuPy’s built-in
cp.is_available() result instead of manually checking HAS_CUPY and querying
getDeviceCount(), while preserving the false result when CuPy is unavailable.
- Around line 19-25: Make the cross-module dependencies imported by gpu_transfer
explicit by renaming _EPSILON, _LAB_AB_BOUNDS, _LAB_L_BOUNDS, and
_meanstd_transfer in transfer_methods and updating every reference, including
gpu_transfer, to their non-underscore names; keep validate_and_resize_images
unchanged.
In `@colorcast/processing/transfer_methods.py`:
- Around line 16-42: Update the _meanstd_transfer docstring to remove the
incorrect requirement that inputs be float32, and describe them as three-channel
floating-point images while preserving the existing shape and spatial-dimension
requirements. Do not add runtime validation or alter the transfer logic or Lab
call sites.
In `@pyproject.toml`:
- Around line 47-57: Consolidate the formatting and import-sorting tooling by
removing the black and isort development dependencies, standardizing on ruff
format and the existing Ruff I lint rules in the Ruff configuration, and
updating CI and the PR checklist to stop invoking isort or black separately.
🪄 Autofix (Beta)
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: 6ef60f84-c4a2-4ad3-a941-6299041d5353
⛔ Files ignored due to path filters (4)
imgs/CVD-Accessibility-Dashboard.pngis excluded by!**/*.pngimgs/Compare-Transfer-Methods.pngis excluded by!**/*.pngimgs/dashboard_report.pngis excluded by!**/*.pngimgs/interface.pngis excluded by!**/*.png
📒 Files selected for processing (68)
.github/ISSUE_TEMPLATE/bug_report.md.github/ISSUE_TEMPLATE/feature_request.md.github/PULL_REQUEST_TEMPLATE.md.github/dependabot.yml.github/workflows/ci.yml.gitignore.zenodo.jsonCHANGELOG.mdCITATION.cffCODE_OF_CONDUCT.mdCONTRIBUTING.mdMANIFEST.inREADME.mdSECURITY.mdcolorcast.pycolorcast/__init__.pycolorcast/__main__.pycolorcast/_version.pycolorcast/analysis/__init__.pycolorcast/analysis/comparison.pycolorcast/analysis/daltonization.pycolorcast/analysis/dashboard.pycolorcast/analysis/error_map.pycolorcast/analysis/visualization.pycolorcast/gui.pycolorcast/processing/__init__.pycolorcast/processing/batch.pycolorcast/processing/blending.pycolorcast/processing/cache.pycolorcast/processing/curves.pycolorcast/processing/gpu_transfer.pycolorcast/processing/image_loader.pycolorcast/processing/registry.pycolorcast/processing/simulation.pycolorcast/processing/transfer_methods.pycolorcast/utils/__init__.pycolorcast/utils/config.pycolorcast/utils/exceptions.pycolorcast/utils/validators.pycolorcast/utils/validators_enhanced.pydocs/conf.pydocs/index.rstpyproject.tomlrequirements-dev.txtrequirements.txtscripts/daltonization_efficacy.pytests/__init__.pytests/test_batch.pytests/test_blending.pytests/test_cache.pytests/test_cli.pytests/test_color_blindness.pytests/test_comparison.pytests/test_config.pytests/test_curves.pytests/test_entry_points.pytests/test_gpu_transfer.pytests/test_gui.pytests/test_image_loading.pytests/test_integration.pytests/test_integration_comprehensive.pytests/test_lab_transfer.pytests/test_performance.pytests/test_property_based.pytests/test_registry.pytests/test_transfer_methods.pytests/test_validators_enhanced.pytests/test_visualization.py
💤 Files with no reviewable changes (4)
- requirements-dev.txt
- colorcast/utils/validators.py
- requirements.txt
- colorcast.py
| def compute_dashboard( | ||
| image_array: np.ndarray, | ||
| max_workers: int | None = None, | ||
| ) -> DashboardResult: | ||
| """Run all three CVD simulations and error maps in parallel. | ||
|
|
||
| Args: | ||
| image_array: Source image, any numeric dtype, shape (H, W, 3). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize original before storing it in the result.
Line 61 documents that image_array accepts any numeric dtype, and ColorBlindSimulator.transform_color_space normalizes internally. Line 90 stores the raw input. DashboardResult.original is documented at Lines 37-38 as float32 in [0, 1].
Both renderers then treat the field as float in [0, 1]:
generate_dashboard_reportLine 166 appliesnp.clip(result.original, 0, 1).colorcast/gui.pyLine 52 applies the same clip in_array_to_pixmap.
For a uint8 input every sample above 1 clamps to 1, so the "Original" panel renders as a saturated image while the simulated panels render correctly. Normalize once in compute_dashboard.
🐛 Proposed fix
+from colorcast.processing.image_loader import normalize_to_float32
from colorcast.processing.simulation import ColorBlindSimulator simulator = ColorBlindSimulator()
+ original = normalize_to_float32(image_array)
def _simulate_and_map(deficiency: str) -> tuple[str, np.ndarray, ErrorMap, dict[str, float]]:
- sim = simulator.transform_color_space(image_array, deficiency)
- em = get_error_map(image_array, sim, compute_dE00=True)
+ sim = simulator.transform_color_space(original, deficiency)
+ em = get_error_map(original, sim, compute_dE00=True) return DashboardResult(
- original=image_array,
+ original=original,Also applies to: 89-94
🤖 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 54 - 61, Normalize image_array
to float32 in the [0, 1] range inside compute_dashboard before assigning it to
DashboardResult.original, using the same normalization behavior as
ColorBlindSimulator.transform_color_space. Keep the normalized original separate
from the raw input used for simulation if needed, and preserve the existing
result structure and parallel processing.
| signed: np.ndarray | ||
| absolute: np.ndarray | ||
| chroma_error: np.ndarray | ||
| chroma_error_dE00: np.ndarray |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find positional ErrorMap construction/unpacking and readers of chroma_error_dE00.
set -euo pipefail
rg -n -C 4 'ErrorMap\(' --type=py
rg -n -C 4 'chroma_error_dE00' --type=py
rg -n -C 3 'get_error_map\(' --type=pyRepository: MichailSemoglou/ColorCast
Length of output: 163
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- tracked Python files ---'
git ls-files '*.py' | sed -n '1,160p'
printf '%s\n' '--- target file ---'
target=$(git ls-files | awk '$0=="colorcast/analysis/error_map.py"{print; exit}')
if [ -n "$target" ]; then
nl -ba "$target" | sed -n '1,280p'
else
printf '%s\n' 'target not found'
fi
printf '%s\n' '--- ErrorMap symbols and consumers ---'
rg -n -C 5 'ErrorMap|chroma_error_dE00|get_error_map' . --glob '*.py' || trueRepository: MichailSemoglou/ColorCast
Length of output: 1605
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- colorcast/analysis/error_map.py ---'
awk '{printf "%6d\t%s\n", NR, $0}' colorcast/analysis/error_map.py | sed -n '1,280p'
printf '%s\n' '--- symbol and call-site search ---'
rg -n -C 5 'ErrorMap|chroma_error_dE00|get_error_map' colorcast tests scripts --glob '*.py' || trueRepository: MichailSemoglou/ColorCast
Length of output: 50381
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
import ast
import subprocess
from pathlib import Path
files = [Path(p) for p in subprocess.check_output(
["git", "ls-files", "*.py"], text=True
).splitlines()]
def dotted_name(node):
parts = []
while isinstance(node, ast.Attribute):
parts.append(node.attr)
node = node.value
if isinstance(node, ast.Name):
parts.append(node.id)
return ".".join(reversed(parts))
return None
print("--- ErrorMap calls with positional arguments ---")
for path in files:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, ast.Call):
name = dotted_name(node.func) or ""
if name == "ErrorMap" or name.endswith(".ErrorMap"):
positional = len(node.args)
keywords = [kw.arg for kw in node.keywords if kw.arg is not None]
if positional or not keywords:
print(f"{path}:{node.lineno}: positional={positional}, keywords={keywords}")
print("--- get_error_map calls and compute_dE00 values ---")
for path in files:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, ast.Call):
name = dotted_name(node.func) or ""
if name == "get_error_map" or name.endswith(".get_error_map"):
positional = len(node.args)
kw = {item.arg: ast.unparse(item.value) for item in node.keywords if item.arg}
print(f"{path}:{node.lineno}: positional={positional}, keywords={kw}")
print("--- direct chroma_error_dE00 readers ---")
for path in files:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, ast.Attribute) and node.attr == "chroma_error_dE00":
print(f"{path}:{node.lineno}: {ast.unparse(node)}")
print("--- tuple unpack assignments fed directly by get_error_map ---")
for path in files:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, (ast.Assign, ast.AnnAssign)):
value = node.value
name = dotted_name(value.func) if isinstance(value, ast.Call) else None
if name == "get_error_map" or (name and name.endswith(".get_error_map")):
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
for target in targets:
if isinstance(target, (ast.Tuple, ast.List)):
print(f"{path}:{node.lineno}: target={ast.unparse(target)}")
PY
printf '%s\n' '--- exact field references ---'
rg -n 'chroma_error_dE00' --glob '*.py' colorcast tests scripts || true
printf '%s\n' '--- ErrorMap constructor references ---'
rg -n -C 3 'ErrorMap\(' --glob '*.py' colorcast tests scripts || trueRepository: MichailSemoglou/ColorCast
Length of output: 5391
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- previous ErrorMap field order ---'
if git rev-parse --verify HEAD^ >/dev/null 2>&1; then
git show HEAD^:colorcast/analysis/error_map.py 2>/dev/null |
awk '/class ErrorMap\(NamedTuple\)/,/^$/' |
grep -E '^[[:space:]]+[A-Za-z_][A-Za-z0-9_]*:'
else
printf '%s\n' 'no parent commit available'
fi
printf '%s\n' '--- possible indexed ErrorMap access ---'
rg -n -C 2 '(error_map|error_maps|em|ErrorMap)[[:space:]]*\[' colorcast tests scripts --glob '*.py' || true
printf '%s\n' '--- public exports ---'
rg -n -C 2 'ErrorMap' colorcast/__init__.py colorcast/analysis/__init__.pyRepository: MichailSemoglou/ColorCast
Length of output: 201
Preserve the public ErrorMap positional contract.
ErrorMap is an exported NamedTuple. This required field changes positional indexes and breaks the previous positional shape for external callers. No repository code constructs or unpacks ErrorMap positionally. The only production reader passes compute_dE00=True; default callers do not read this field. Keep the optional metric outside the existing positional tuple contract.
🤖 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` at line 93, Update the exported ErrorMap
NamedTuple definition to keep its existing positional fields and indexes
unchanged; do not add chroma_error_dE00 as a required tuple field. Store the
optional dE00 metric outside the positional tuple contract while preserving
compute_dE00=True behavior for the production reader.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
97-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the mypy suppressions.
These settings suppress more than PyQt5 stub errors:
warn_return_any = falsedisables return-Anywarnings globally,ignore_missing_imports = truehides unresolved imports globally, andignore_errors = trueskips every diagnostic incolorcast.gui. The new GUI code is therefore outside the type-checking guarantee. Keepcolorcast.guichecked and scope suppressions to the Qt modules or specific diagnostics.🤖 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 `@pyproject.toml` around lines 97 - 103, Scope the mypy configuration suppressions in the main settings and colorcast.gui override: remove the global warn_return_any and ignore_missing_imports disables, keep colorcast.gui type-checked by removing ignore_errors, and apply narrowly targeted overrides only to the relevant PyQt5/Qt modules or specific diagnostics.
🤖 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 `@pyproject.toml`:
- Line 96: Align mypy with the project’s minimum supported Python version by
changing the mypy target in pyproject.toml and the corresponding CI job
configuration in .github/workflows/ci.yml from Python 3.12 to Python 3.10, or
configure mypy to run across every supported version.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 97-103: Scope the mypy configuration suppressions in the main
settings and colorcast.gui override: remove the global warn_return_any and
ignore_missing_imports disables, keep colorcast.gui type-checked by removing
ignore_errors, and apply narrowly targeted overrides only to the relevant
PyQt5/Qt modules or specific diagnostics.
🪄 Autofix (Beta)
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: a3ea2fb0-2bdb-4858-b6c5-6c7b1f86c2bb
📒 Files selected for processing (7)
.github/workflows/ci.ymlcolorcast/analysis/comparison.pycolorcast/analysis/daltonization.pycolorcast/analysis/dashboard.pycolorcast/processing/image_loader.pycolorcast/utils/validators_enhanced.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- colorcast/processing/image_loader.py
- colorcast/analysis/daltonization.py
- colorcast/utils/validators_enhanced.py
- colorcast/analysis/dashboard.py
- colorcast/analysis/comparison.py
| [tool.mypy] | ||
| python_version = "3.10" | ||
| warn_return_any = true | ||
| python_version = "3.12" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'requires-python|python_version|python-version' pyproject.toml .github/workflows/ci.ymlRepository: MichailSemoglou/ColorCast
Length of output: 539
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- pyproject.toml ---'
sed -n '1,115p' pyproject.toml
printf '%s\n' '--- .github/workflows/ci.yml ---'
sed -n '1,80p' .github/workflows/ci.yml
printf '%s\n' '--- parsed version settings ---'
python3 - <<'PY'
import pathlib, re, tomllib
pyproject = tomllib.loads(pathlib.Path("pyproject.toml").read_text())
print("requires-python:", pyproject["project"].get("requires-python"))
print("tool.mypy.python_version:", pyproject.get("tool", {}).get("mypy", {}).get("python_version"))
workflow = pathlib.Path(".github/workflows/ci.yml").read_text()
for pattern in (
r'python-version:\s*"([^"]+)"',
r'python-version:\s*\[(.*?)\]',
):
print(pattern, re.findall(pattern, workflow))
PYRepository: MichailSemoglou/ColorCast
Length of output: 5068
Align mypy with the minimum supported Python version.
pyproject.toml declares Python >=3.10, but mypy targets and runs on Python 3.12. Set the mypy target and CI job to Python 3.10, or run mypy for every supported version.
📍 Affects 2 files
pyproject.toml#L96-L96(this comment).github/workflows/ci.yml#L39-L43
🤖 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 `@pyproject.toml` at line 96, Align mypy with the project’s minimum supported
Python version by changing the mypy target in pyproject.toml and the
corresponding CI job configuration in .github/workflows/ci.yml from Python 3.12
to Python 3.10, or configure mypy to run across every supported version.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
colorcast/gui.py (4)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exporting public aliases instead of importing private names.
_DEFICIENCIESand_DEFICIENCY_LABELSare private tocolorcast.analysis.dashboard. The GUI depends on them across module boundaries, so renames insidedashboard.pywill break the GUI silently. Export public aliases (for exampleDEFICIENCIESandDEFICIENCY_LABELS) and import those.🤖 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 28 - 33, In colorcast.analysis.dashboard, expose public aliases for the private _DEFICIENCIES and _DEFICIENCY_LABELS symbols, then update the GUI import to use DEFICIENCIES and DEFICIENCY_LABELS while preserving existing behavior.
747-758: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRecord the failure reason and reuse the transfer cache.
Two points in this loop:
- Line 755-756 discards the exception. The log record contains only the method id, so a failing method cannot be diagnosed from the logs. Add
exc_info=True.- Line 752 calls
instance.transfer(...)directly. The main window usesregistry.transfer_cachedat line 416. The dialog therefore recomputes transfers that are already cached for the same content and style images. Useregistry.transfer_cachedto reuse those results.♻️ Proposed change
def _start_computation(self) -> None: def _run() -> None: for method_id in self._label_widgets: try: - instance = registry.get_method(method_id) - self._results[method_id] = instance.transfer( - self._content_image, self._style_image - ) + self._results[method_id] = registry.transfer_cached( + method_id, self._content_image, self._style_image + ) except Exception: # noqa: BLE001 — skip unsupported methods - logger.debug("Skipped method %s in comparison", method_id) + logger.debug( + "Skipped method %s in comparison", method_id, exc_info=True + )🤖 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 747 - 758, Update the _start_computation loop to call registry.transfer_cached with the method identifier and current content/style images instead of instance.transfer, preserving result storage in self._results. Enhance the skipped-method logger.debug call with exc_info=True so the caught exception details are recorded.
760-776: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the completion count.
self._resultsonly receives successful arrays, so thev is not Nonefilter at line 774 is always true. Uselen(self._results).♻️ Proposed change
- done = sum(1 for v in self._results.values() if v is not None) + done = len(self._results)🤖 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 760 - 776, In _on_computation_done, simplify the completed-method count by using len(self._results) directly instead of summing values filtered by v is not None; leave the total calculation and status text unchanged.
311-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared image-file filter. Define it once and reuse it in both dialogs.
ALLOWED_IMAGE_EXTENSIONSis already a dotted tuple with stable order, so sorting is unnecessary.🤖 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 311 - 325, Extract the repeated image-file filter expression into a shared value, then reuse it in both file dialogs within the image-loading methods. Build it directly from ALLOWED_IMAGE_EXTENSIONS in its existing order without sorting, and preserve the current dialog behavior and titles.
🤖 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/gui.py`:
- Around line 71-85: Update _run_in_thread and _WorkerSignals usage so the
signals QObject remains strongly referenced until the queued finished callback
has been delivered, rather than only through the worker closure. Release that
retained reference after delivery completes while preserving the existing
on_done callback behavior.
In `@colorcast/utils/config.py`:
- Around line 102-106: Validate that the parsed JSON value in the
configuration-loading flow is an object/mapping before the loop over
raw.items(). Raise TypeError for roots such as arrays or null, while preserving
the existing field filtering behavior for valid object roots.
---
Nitpick comments:
In `@colorcast/gui.py`:
- Around line 28-33: In colorcast.analysis.dashboard, expose public aliases for
the private _DEFICIENCIES and _DEFICIENCY_LABELS symbols, then update the GUI
import to use DEFICIENCIES and DEFICIENCY_LABELS while preserving existing
behavior.
- Around line 747-758: Update the _start_computation loop to call
registry.transfer_cached with the method identifier and current content/style
images instead of instance.transfer, preserving result storage in self._results.
Enhance the skipped-method logger.debug call with exc_info=True so the caught
exception details are recorded.
- Around line 760-776: In _on_computation_done, simplify the completed-method
count by using len(self._results) directly instead of summing values filtered by
v is not None; leave the total calculation and status text unchanged.
- Around line 311-325: Extract the repeated image-file filter expression into a
shared value, then reuse it in both file dialogs within the image-loading
methods. Build it directly from ALLOWED_IMAGE_EXTENSIONS in its existing order
without sorting, and preserve the current dialog behavior and titles.
🪄 Autofix (Beta)
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: 08165e77-f51c-452f-97b8-71c19fbb4c98
⛔ Files ignored due to path filters (4)
imgs/CVD-Accessibility-Dashboard.pngis excluded by!**/*.pngimgs/Compare-Transfer-Methods.pngis excluded by!**/*.pngimgs/dashboard_report.pngis excluded by!**/*.pngimgs/interface.pngis excluded by!**/*.png
📒 Files selected for processing (68)
.github/ISSUE_TEMPLATE/bug_report.md.github/ISSUE_TEMPLATE/feature_request.md.github/PULL_REQUEST_TEMPLATE.md.github/dependabot.yml.github/workflows/ci.yml.gitignore.zenodo.jsonCHANGELOG.mdCITATION.cffCODE_OF_CONDUCT.mdCONTRIBUTING.mdMANIFEST.inREADME.mdSECURITY.mdcolorcast.pycolorcast/__init__.pycolorcast/__main__.pycolorcast/_version.pycolorcast/analysis/__init__.pycolorcast/analysis/comparison.pycolorcast/analysis/daltonization.pycolorcast/analysis/dashboard.pycolorcast/analysis/error_map.pycolorcast/analysis/visualization.pycolorcast/gui.pycolorcast/processing/__init__.pycolorcast/processing/batch.pycolorcast/processing/blending.pycolorcast/processing/cache.pycolorcast/processing/curves.pycolorcast/processing/gpu_transfer.pycolorcast/processing/image_loader.pycolorcast/processing/registry.pycolorcast/processing/simulation.pycolorcast/processing/transfer_methods.pycolorcast/utils/__init__.pycolorcast/utils/config.pycolorcast/utils/exceptions.pycolorcast/utils/validators.pycolorcast/utils/validators_enhanced.pydocs/conf.pydocs/index.rstpyproject.tomlrequirements-dev.txtrequirements.txtscripts/daltonization_efficacy.pytests/__init__.pytests/test_batch.pytests/test_blending.pytests/test_cache.pytests/test_cli.pytests/test_color_blindness.pytests/test_comparison.pytests/test_config.pytests/test_curves.pytests/test_entry_points.pytests/test_gpu_transfer.pytests/test_gui.pytests/test_image_loading.pytests/test_integration.pytests/test_integration_comprehensive.pytests/test_lab_transfer.pytests/test_performance.pytests/test_property_based.pytests/test_registry.pytests/test_transfer_methods.pytests/test_validators_enhanced.pytests/test_visualization.py
💤 Files with no reviewable changes (4)
- requirements.txt
- colorcast/utils/validators.py
- colorcast.py
- requirements-dev.txt
🚧 Files skipped from review as they are similar to previous changes (59)
- .gitignore
- tests/init.py
- CITATION.cff
- colorcast/processing/curves.py
- colorcast/processing/blending.py
- colorcast/processing/init.py
- tests/test_cli.py
- .github/ISSUE_TEMPLATE/feature_request.md
- tests/test_registry.py
- colorcast/utils/init.py
- colorcast/analysis/init.py
- tests/test_blending.py
- tests/test_integration.py
- tests/test_curves.py
- CONTRIBUTING.md
- scripts/daltonization_efficacy.py
- colorcast/_version.py
- .github/ISSUE_TEMPLATE/bug_report.md
- SECURITY.md
- pyproject.toml
- colorcast/processing/transfer_methods.py
- tests/test_visualization.py
- docs/conf.py
- .github/PULL_REQUEST_TEMPLATE.md
- MANIFEST.in
- CODE_OF_CONDUCT.md
- tests/test_lab_transfer.py
- colorcast/utils/exceptions.py
- tests/test_validators_enhanced.py
- .zenodo.json
- tests/test_color_blindness.py
- tests/test_gui.py
- colorcast/processing/image_loader.py
- tests/test_comparison.py
- tests/test_integration_comprehensive.py
- tests/test_image_loading.py
- tests/test_transfer_methods.py
- colorcast/init.py
- .github/dependabot.yml
- colorcast/processing/simulation.py
- .github/workflows/ci.yml
- tests/test_cache.py
- tests/test_batch.py
- colorcast/analysis/dashboard.py
- tests/test_gpu_transfer.py
- colorcast/analysis/comparison.py
- tests/test_property_based.py
- colorcast/utils/validators_enhanced.py
- colorcast/processing/cache.py
- colorcast/analysis/daltonization.py
- colorcast/processing/batch.py
- tests/test_performance.py
- colorcast/processing/gpu_transfer.py
- colorcast/analysis/error_map.py
- README.md
- colorcast/analysis/visualization.py
- colorcast/processing/registry.py
- docs/index.rst
- colorcast/main.py
Replace eager imports of MethodComparison, daltonize, ErrorMap, and get_error_map with module-level __getattr__ that resolves them on first access. Importing colorcast no longer initializes colorcast.analysis or requires matplotlib.
- ErrorMap.chroma_error_dE00 made optional (defaults to None, not NaN array) - daltonization docs: clarify L* is preserved from base, not restored from orig - error_map docs: remove stale claim about apply_daltonization using orig_l_star - dashboard: normalize image_array to float32 before storing in DashboardResult - gui: keep QObject reference alive until signal is delivered - gui: emit finished signal in finally block - config: raise TypeError on non-object JSON roots - SECURITY.md: mark 2.5.x as supported - tests: update dE00 assertions for optional field - tests: add Figure assertion to test_custom_title - docs: remove orphaned wiki markdown file
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
colorcast/utils/config.py (3)
53-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat empty environment variables as unset.
Use
os.environ.get("APPDATA") or ...andos.environ.get("XDG_CONFIG_HOME") or ...so empty values do not resolve to the current directory.🤖 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/utils/config.py` around lines 53 - 55, Update the base-path assignments in the configuration path logic to treat empty APPDATA and XDG_CONFIG_HOME values as unset by falling back to their existing platform-specific defaults. Preserve the current Path conversion and fallback locations.
36-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not stringify unsupported configuration values.
default=strconverts non-JSON values incustom_methodsto strings.load()accepts those strings, so the original plugin values cannot be restored. Removedefault=stror implement an explicit serializer for supported plugin values.🤖 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/utils/config.py` at line 36, Update the custom_methods configuration serialization in the relevant load/save configuration logic to remove default=str, or replace it with an explicit serializer limited to supported plugin values. Ensure load() preserves and restores supported non-JSON plugin values instead of accepting irreversible stringified representations.
111-127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate semantic configuration values before returning
ColorCastConfig.
load()accepts negative dimensions and debounce intervals,default_intensityoutside[0.0, 1.0], and unregistereddefault_methodvalues.StyleTransferAppuses these values directly, and an invalid method causesregistry.get_method()to raiseValueError. Add field-specific bounds and validatedefault_methodagainst registered method IDs.🤖 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/utils/config.py` around lines 111 - 127, Update the configuration validation in load() before constructing ColorCastConfig: require dimensions and debounce intervals to be non-negative, constrain default_intensity to [0.0, 1.0], and ensure default_method matches a registered method ID. Preserve the existing type validation and raise a clear configuration error for each invalid semantic value.
🤖 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.
Outside diff comments:
In `@colorcast/utils/config.py`:
- Around line 53-55: Update the base-path assignments in the configuration path
logic to treat empty APPDATA and XDG_CONFIG_HOME values as unset by falling back
to their existing platform-specific defaults. Preserve the current Path
conversion and fallback locations.
- Line 36: Update the custom_methods configuration serialization in the relevant
load/save configuration logic to remove default=str, or replace it with an
explicit serializer limited to supported plugin values. Ensure load() preserves
and restores supported non-JSON plugin values instead of accepting irreversible
stringified representations.
- Around line 111-127: Update the configuration validation in load() before
constructing ColorCastConfig: require dimensions and debounce intervals to be
non-negative, constrain default_intensity to [0.0, 1.0], and ensure
default_method matches a registered method ID. Preserve the existing type
validation and raise a clear configuration error for each invalid semantic
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05169881-2d84-4b90-99a1-235a7ebbbac3
📒 Files selected for processing (11)
CHANGELOG.mdREADME.mdSECURITY.mdcolorcast/analysis/daltonization.pycolorcast/analysis/dashboard.pycolorcast/analysis/error_map.pycolorcast/gui.pycolorcast/utils/config.pydocs/wiki/Color-Vision-Background.mdtests/test_color_blindness.pytests/test_visualization.py
💤 Files with no reviewable changes (1)
- docs/wiki/Color-Vision-Background.md
🚧 Files skipped from review as they are similar to previous changes (8)
- SECURITY.md
- tests/test_color_blindness.py
- colorcast/analysis/daltonization.py
- CHANGELOG.md
- README.md
- colorcast/analysis/error_map.py
- colorcast/analysis/dashboard.py
- colorcast/gui.py
1 participant
What's new
Added
compute_dashboard,DashboardResult,generate_dashboard_report) for comparing all three deficiencies at onceQT_QPA_PLATFORM=offscreen--verboseflag on the CLI parser and each subcommand so tracebacks on errors are opt-inpyproject.tomlChanged
method.parameters, preventing signature clashes with reference-free methodscolorcast/_version.py;pyproject.tomlusesdynamic = ["version"]reading from itgpu_histogram_matchingcollapsed to CPU body; CuPy import warning removed;is_gpu_available()is the public entry point_EPSILON,_LAB_L_BOUNDS,_LAB_AB_BOUNDS) eliminating 6 copiesStyleTransferAppderives settings fromColorCastConfig; file-dialog filters fromALLOWED_IMAGE_EXTENSIONSapply_daltonizationrefactored with_compute_chromaticity_weighthelpervalidate_and_resize_imagesreturn annotation tightenedbatch.pycatchesFileNotFoundErrorandOSError__init__.pyre-exports CVD and Daltonization symbolspylintandpytest-mock; addedimageioFixed
daltonize(intensity=0)now returns the original image instead of the simulated imageselective_color_transferuses smoothstep masks over a ±0.05 luminance band, eliminating hard seamssave_imagere-raises withfrom eto preserve the cause chainshow_imagecopiesQImagebeforeQPixmapto prevent GC crashesStyleTransferCacheis now thread-safe withthreading.Lockvalidate_file_pathusesPath.relative_toinstead ofstr.startswithto reject sibling-prefix bypass_compute_hashuses downsampled fingerprint instead of hashing full image bytesMethodComparison.find_best_methodinfers metric direction automaticallyColorCastConfig.load()validates types;enable_parallelwired toBatchProcessorRemoved
gpu_histogram_matchingColorCastConfig.cache_sizeunused fieldrequirements.txtandrequirements-dev.txtcolorcast.pyandcolorcast/utils/validators.pyTesting
python -m buildandtwine checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation