AtlasPatch + other additions - #43
Open
polejowska wants to merge 8 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands segmenteer’s segmentation method catalog (notably adding SAM3 and AtlasPatch+SAM2 options) and improves benchmarking/reporting ergonomics to better support experimentation with “segment anything” style workflows.
Changes:
- Add new DL segmenters (SAM3, AtlasPatch+SAM2) and expose them through the public API.
- Add new classical segmenters (connected components, edge-based) and adjust Otsu thresholding behavior.
- Enhance benchmarking/report output (optional thumbnail saving, prompt persistence, and report thumbnail fallback generation) and make TRIDENT imports lazier.
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| segmenteer/methods/ensemble/init.py | Formatting/structure tweaks in ensemble utilities. |
| segmenteer/methods/dl/trident.py | Import formatting; integrates PathProfiler TRIDENT wrapper change. |
| segmenteer/methods/dl/sam3.py | New SAM3Segmenter implementation (Ultralytics SAM3). |
| segmenteer/methods/dl/rtlucassen.py | Minor formatting change. |
| segmenteer/methods/dl/pathprofiler.py | Refactor to lazy TRIDENT loading via dynamic class creation. |
| segmenteer/methods/dl/grandqc.py | Uses parenthesized context manager for tqdm/file handling. |
| segmenteer/methods/dl/fastsam.py | Return type adjusted to boolean mask (NumpySegmenter contract). |
| segmenteer/methods/dl/atlaspatch_sam2.py | New AtlasPatchSAM2Segmenter integrating atlas-patch + SAM2. |
| segmenteer/methods/dl/init.py | Export new DL segmenters with optional-import pattern. |
| segmenteer/methods/classical/threshold.py | Fix Otsu direction; add new classical segmenters. |
| segmenteer/methods/classical/background_subtractor.py | Minor formatting change. |
| segmenteer/methods/classical/init.py | Export new classical segmenters. |
| segmenteer/methods/init.py | Re-export new classical methods and reorder imports. |
| segmenteer/core/base.py | Add lazy TRIDENT import helper and adjust TRIDENTSegmenter. |
| segmenteer/benchmark/workflows.py | Add save_thumbnails option for single/dataset benchmark runs. |
| segmenteer/benchmark/runner.py | Extend BenchmarkResult with prompt; attempt to capture it. |
| segmenteer/benchmark/ensemble.py | Persist prompts per image; formatting refactors. |
| segmenteer/init.py | Expose new segmenters at top-level via getattr and all. |
| run.py | Add AtlasPatchSAM2Segmenter to example list; disable thumbnails in dataset run. |
| README.md | Document FastSAM text-prompt dependency caveats for uv/pip installs. |
| pyproject.toml | Add sam3 and atlaspatch-sam2 extras; adjust dl/fastsam extras. |
| postensemble.py | Minor formatting. |
| docs/fastsam.md | New FastSAM documentation page. |
| docs/classical_methods.md | New documentation for classical segmentation methods. |
| create_report.py | Minor formatting for readability. |
| app/report.py | Add on-the-fly thumbnail generation fallback in report output. |
| app/loader.py | Minor formatting. |
| .gitignore | Ignore runs/ directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
siemdejong
requested changes
Apr 30, 2026
siemdejong
left a comment
siemdejong
left a comment
Contributor
There was a problem hiding this comment.
Looks good overall, agata!
| # Filter components by minimum area | ||
| mask = np.zeros_like(binary) | ||
| for component_id in np.unique(labeled): | ||
| if component_id == 0: # Skip background |
Contributor
There was a problem hiding this comment.
Just checking: are we sure that component 0 is always background?
| # Build mask from components, filtering by minimum area | ||
| mask = np.zeros_like(inverted_edges) | ||
| for component_id in np.unique(labeled): | ||
| if component_id == 0: # Skip background (edges) |
|
|
||
| if not results or len(results) == 0: | ||
| return mask_to_geojson(np.zeros(image.shape[:2], dtype=bool), self.min_area) | ||
| return np.zeros(image.shape[:2], dtype=bool) |
| TRIDENTSegmentationModel, _ = _require_trident() | ||
|
|
||
| # Dynamically create the actual class with TRIDENT base | ||
| class _PathProfilerImpl(TRIDENTSegmentationModel): |
Contributor
There was a problem hiding this comment.
This part reads really really hard. Not many people understand what __new__ does and this __new__ method in particular defines its own class.
What is the rationale behind this change? It looks like a Copilot suggestion and correlates with the Copilot comment below. Maybe we should indeed use string annotations here or put the _TORCH_AVAILABLE guard before the LIBTRIDENTPathProfilerSegmenter definition.
I'm also fine with keeping the code as-is, but with some comments why this is structured like this.
| seg.run_ensemble( | ||
| manifest_path=None, # or: Path("outputs/.../ensemble_manifest.json") | ||
| threshold=0.5, # 0.0 = union | 0.5 = majority | 1.0 = intersection | ||
| threshold=0.5, # 0.0 = union | 0.5 = majority | 1.0 = intersection |
Contributor
There was a problem hiding this comment.
I find this comment a bit confusing, especially because the threshold actually means what is in the brackets below which is clearer in my opinion. I know this is not really a change in this PR, but it only now caught my eye.
* threshold = 0.0 → union (any method wins)
* threshold = 0.5 → majority (>50 % weighted agreement)
* threshold = 1.0 → intersection (all methods must agree)| # all deep learning methods dependecies combined | ||
| # (bigpicture excluded: tissue-segmentation requires tensorflow-cpu, no macOS ARM wheel) | ||
| dl = [ | ||
| "atlas-patch", |
Contributor
There was a problem hiding this comment.
Shall we pin all atlas-patch to >=1.1.0?
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
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.
#24: after experimentation with "2stage_pipeline.py", currently SAM working in a segment anything mode demonstrates the most promising results compared to the text-based prompting.
Classical methods (also those newly added) refinement (parameters tuning and review) will be be covered by #38