Repository navigation
refactor: gitlab to github migration - #4
TomasBalak wants to merge 4 commits into
Conversation
- Replace GitLab CI with GitHub Actions workflows - Migrate from pdm to uv - Update pre-commit config, ruff, mypy, and markdownlint configs - Update documentation URLs from GitLab to GitHub - Reorganize pyproject.toml for uv compatibility
- Fix import ordering across all modules - Fix folding threshold calculation for empty masks - Apply ruff auto-fixes for consistency
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the project's development environment and dependency management from PDM to uv, updates pre-commit and linter configurations, and revises the documentation and example code. It also refactors the threshold calculation logic in folding.py and standardizes imports across several modules. The review feedback highlights several critical issues in the updated documentation where incorrect dictionary keys are used (which would cause KeyErrors at runtime), suggests falling back to the tile's own tissue mask in folding.py when the local neighborhood is empty, recommends raising a ValueError for mismatched local parameters, and points out redundant imports in blur_score_laplacian.py.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
📝 WalkthroughWalkthroughThis PR migrates the project from PDM to UV as the dependency manager, updates CI/CD infrastructure to use GitHub Actions with shared templates, refines pre-commit and linting configurations, and comprehensively updates documentation and examples. Minor code improvements enhance type safety in the folding module and consolidate imports across QC functions. Version bumped to 1.0.1. ChangesBuild System and Tooling Migration
Documentation, Examples, and Code Quality
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TomasBalak
changed the title
Both ColorConversion and convert_color are now imported from rationai.staining, consistent with the rest of the codebase.
![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 (2)
rationai/qc/folding/folding.py (2)
119-121:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix dtype mismatch in local_eosin_channel initialization.
Line 121 assigns
np.ones_like(local_tiles)tolocal_eosin_channel, butlocal_tilesis an RGB image with dtype uint8, while the parameter is typed asNDArray[np.float64] | None. The mypy error confirms this type mismatch.🔧 Proposed fix
else: eosin_channel = np.ones_like(tissue_mask) if local_tiles is not None: - local_eosin_channel = np.ones_like(local_tiles) + local_eosin_channel = np.ones(local_tiles.shape[:2], dtype=np.float64)Note: Use
shape[:2]to get height×width only, since eosin_channel is 2D while local_tiles is 3D (RGB).🤖 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 `@rationai/qc/folding/folding.py` around lines 119 - 121, The local_eosin_channel is created from local_tiles which is an RGB uint8 array causing a dtype mismatch; instead allocate a 2D float array matching the spatial shape of local_tiles (height×width) and dtype float64 like eosin_channel/tissue_mask. Replace the np.ones_like(local_tiles) usage by creating an array using local_tiles.shape[:2] (or np.ones_like(tissue_mask)) with dtype=np.float64 so local_eosin_channel has the correct 2D shape and type; update references to local_eosin_channel accordingly.Source: Pipeline failures
154-169:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEliminate variable redefinition and simplify control flow.
The variable
resultis defined with type annotation on both line 156 and line 163, causing a mypy redefinition error. Both branches also construct nearly identical dictionaries, differing only in the value offolding_test.♻️ Proposed refactoring
if hematoxylin_eosin_stained: folding_test = reconstruction(folding_test_markers, thresholded_eosin) - result: FoldArtifacts = { - "folding": folding_test, - "thresholded_saturation": thresholded_saturation, - "thresholded_eosin": thresholded_eosin, - "thresholded_value": thresholded_value, - } - return result - result: FoldArtifacts = { + else: + folding_test = folding_test_markers + + result: FoldArtifacts = { "folding": folding_test, "thresholded_saturation": thresholded_saturation, "thresholded_eosin": thresholded_eosin, "thresholded_value": thresholded_value, } + return result🤖 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 `@rationai/qc/folding/folding.py` around lines 154 - 169, The code duplicates the FoldArtifacts dict and redefines `result` twice, causing a mypy redefinition error; fix by computing `folding_test` conditionally (e.g., set `folding_test = reconstruction(folding_test_markers, thresholded_eosin)` only if `hematoxylin_eosin_stained` else keep existing value or None) and then build and return a single `result: FoldArtifacts = { "folding": folding_test, "thresholded_saturation": thresholded_saturation, "thresholded_eosin": thresholded_eosin, "thresholded_value": thresholded_value }` once; update the code paths in the folding function so only `folding_test` is chosen per branch and the `result` construction/return is unified.Source: Pipeline failures
🧹 Nitpick comments (3)
.github/workflows/mkdocs-build.yml (1)
11-11: Shared workflow@mainreference appears intentional in this repo.Both
.github/workflows/mkdocs-build.ymland.github/workflows/python-lint.ymldelegate toRationAI/.github/.github/workflows/*@main``, indicating the@main(unpinned) pattern is used consistently for “RationAI Standard” shared workflows. Supply-chain risk remains as a design tradeoff; pinning to a commit SHA should be considered only if required by your security policy.🤖 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/mkdocs-build.yml at line 11, The workflow reference "RationAI/.github/.github/workflows/mkdocs-build.yml@main" is intentionally unpinned; decide whether to leave it as `@main` or pin to a specific commit SHA/tag for supply-chain risk reduction. If you must pin, replace the `@main` suffix with a vetted commit SHA or release tag in the reference string (and make the same change for the analogous "RationAI/.github/.github/workflows/python-lint.yml@main" reference), run the CI to validate the included shared workflows still resolve and behave as expected, and update repository docs/policy noting the chosen pinning strategy.Source: Linters/SAST tools
docs/getting-started/installation.md (1)
21-36: ⚡ Quick winConsider documenting the Python version requirement.
The installation instructions work correctly, but the project requires Python >=3.11 (as declared in pyproject.toml). Adding a note about this requirement would help users avoid installation failures due to Python version mismatches.
📋 Suggested addition
Add a brief note before or after the installation methods:
!!! note "Python Version" This library requires Python 3.11 or higher.🤖 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 `@docs/getting-started/installation.md` around lines 21 - 36, Add a short note about the Python version requirement near the "### Install the Core Quality Control Library" section: state that the library requires Python 3.11 or higher (as declared in pyproject.toml) and place this note immediately before or after the listed installation methods so users see it before attempting installation.rationai/qc/blur/blur_score_laplacian.py (1)
5-5: 💤 Low valueMypy stub incompleteness for skimage.morphology (informational).
The pipeline shows mypy errors for
binary_dilationandbinary_erosionnot being explicitly exported. These functions are available in scikit-image 0.24.0 at runtime but may be missing from the type stubs. This is a mypy tooling limitation rather than a code issue.If you want to suppress these specific mypy errors, you can add
# type: ignore[attr-defined]comments, but the code itself is correct.🤖 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 `@rationai/qc/blur/blur_score_laplacian.py` at line 5, Mypy reports missing attributes for skimage.morphology exports; silence this tool-only error by annotating the import of binary_dilation and binary_erosion with a type ignore for undefined attributes (add "# type: ignore[attr-defined]" to the import line that brings in binary_dilation and binary_erosion) so the runtime-available functions binary_dilation and binary_erosion can be used without failing static checks.Source: Pipeline failures
🤖 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 `@docs/getting-started/quality-control.md`:
- Around line 238-240: The code uses the wrong dict key for the FoldArtifacts
output; change the access to use the TypedDict's defined key "folding" instead
of "folding_per_pixel" (i.e., update the expression that builds mask from
artifacts[...] so Image.fromarray receives artifacts["folding"].astype(np.uint8)
scaled appropriately); look for the mask creation line that calls
Image.fromarray and update the key reference to "folding".
- Around line 56-62: The example uses wrong dictionary keys from the
ResidualArtifacts TypedDict; replace any use of artifacts["artifacts_per_pixel"]
with artifacts["coverage_mask"] and stop referencing non-existent keys
artifacts['number_of_flagged_pixels'] and
artifacts['number_of_examined_pixels']; instead use the provided
artifacts["coverage"] value and the coverage_mask to compute or describe
coverage as needed. Update the explanatory text that mentions "examined pixels"
and "flagged pixels" to refer to the API's terms ("coverage_mask" and
"coverage") or to derived values computed from those two fields (e.g., percent
coverage) so documentation matches the ResidualArtifacts contract.
In `@pyproject.toml`:
- Around line 19-20: The pyproject.toml still lists git dependencies pointing to
GitLab for the rationai-masks and rationai-staining packages; update these
entries (the strings referencing "rationai-masks @
git+https://gitlab.ics.muni.cz/..." and "rationai-staining @
git+https://gitlab.ics.muni.cz/...") to the correct GitHub repository URLs if
those packages were migrated (or else update the PR description/docs to
explicitly state these two dependencies remain on GitLab); locate the dependency
lines in pyproject.toml and replace the GitLab URLs with the authoritative
GitHub repo addresses for RationAI/masks and RationAI/staining (or add a clear
comment in the PR/docs clarifying they intentionally remain on GitLab).
In `@rationai/qc/staining/color_difference.py`:
- Line 3: Replace the incorrect import and field accesses to match the updated
rationai-staining API: change the import in this module to import
ConversionDirection (not ConversionType) alongside ColorConversion, and update
all uses of conversion.conv_type to conversion.direction and
conversion.value[...] to use conversion.matrix or conversion.inverse as
appropriate; specifically inspect code paths that expect RGB2STAIN/STAIN2RGB and
switch logic to read conversion.direction, and pull the transformation arrays
from conversion.matrix (or the inverse matrix from conversion.inverse) instead
of conversion.value.
---
Outside diff comments:
In `@rationai/qc/folding/folding.py`:
- Around line 119-121: The local_eosin_channel is created from local_tiles which
is an RGB uint8 array causing a dtype mismatch; instead allocate a 2D float
array matching the spatial shape of local_tiles (height×width) and dtype float64
like eosin_channel/tissue_mask. Replace the np.ones_like(local_tiles) usage by
creating an array using local_tiles.shape[:2] (or np.ones_like(tissue_mask))
with dtype=np.float64 so local_eosin_channel has the correct 2D shape and type;
update references to local_eosin_channel accordingly.
- Around line 154-169: The code duplicates the FoldArtifacts dict and redefines
`result` twice, causing a mypy redefinition error; fix by computing
`folding_test` conditionally (e.g., set `folding_test =
reconstruction(folding_test_markers, thresholded_eosin)` only if
`hematoxylin_eosin_stained` else keep existing value or None) and then build and
return a single `result: FoldArtifacts = { "folding": folding_test,
"thresholded_saturation": thresholded_saturation, "thresholded_eosin":
thresholded_eosin, "thresholded_value": thresholded_value }` once; update the
code paths in the folding function so only `folding_test` is chosen per branch
and the `result` construction/return is unified.
---
Nitpick comments:
In @.github/workflows/mkdocs-build.yml:
- Line 11: The workflow reference
"RationAI/.github/.github/workflows/mkdocs-build.yml@main" is intentionally
unpinned; decide whether to leave it as `@main` or pin to a specific commit
SHA/tag for supply-chain risk reduction. If you must pin, replace the `@main`
suffix with a vetted commit SHA or release tag in the reference string (and make
the same change for the analogous
"RationAI/.github/.github/workflows/python-lint.yml@main" reference), run the CI
to validate the included shared workflows still resolve and behave as expected,
and update repository docs/policy noting the chosen pinning strategy.
In `@docs/getting-started/installation.md`:
- Around line 21-36: Add a short note about the Python version requirement near
the "### Install the Core Quality Control Library" section: state that the
library requires Python 3.11 or higher (as declared in pyproject.toml) and place
this note immediately before or after the listed installation methods so users
see it before attempting installation.
In `@rationai/qc/blur/blur_score_laplacian.py`:
- Line 5: Mypy reports missing attributes for skimage.morphology exports;
silence this tool-only error by annotating the import of binary_dilation and
binary_erosion with a type ignore for undefined attributes (add "# type:
ignore[attr-defined]" to the import line that brings in binary_dilation and
binary_erosion) so the runtime-available functions binary_dilation and
binary_erosion can be used without failing static checks.
🪄 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: ffda644f-4c55-42f0-ac37-59585255edac
⛔ Files ignored due to path filters (2)
pdm.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/workflows/mkdocs-build.yml.github/workflows/python-lint.yml.gitlab-ci.yml.markdownlint.yaml.mypy.ini.pre-commit-config.yaml.ruff.tomlREADME.mddocs/getting-started/installation.mddocs/getting-started/quality-control.mddocs/index.mdpyproject.tomlrationai/qc/blur/blur_score_laplacian.pyrationai/qc/folding/folding.pyrationai/qc/residual_artifacts/residual_artifacts_and_coverage.pyrationai/qc/staining/color_difference.pyrationai/qc/staining/dominant_stains.pyrationai/qc/staining/staining_difference.py
💤 Files with no reviewable changes (1)
- .gitlab-ci.yml
Summary
This PR migrates the CI/CD pipeline from GitLab to GitHub and applies necessary
linting fixes to ensure the merge pipeline passes. It also includes AI
review-suggested fixes for pre-existing code in
master(suggestions originate from #2).Note
This PR is a part of a two-stage PR based on #2. While there might be some issues in this PR, they should be fixed after the PR #5 is merged. The original PR was split in order to be more "review-friendly."
Changes
CI/CD Migration
Build System & Config
pdmtouvDocumentation
docs/getting-started/quality-control.mdLinting & AI Review Fixes (Existing Code)
ruffin the existing code_get_threshold()infolding/folding.pyto handle empty masks (according to AI review in refactor: residual coverage, uv, and migration #2)Summary by CodeRabbit
Documentation
Chores