Skip to content

✨ Define SemanticSegmentor with the New EngineABC #866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 63 commits into
base: dev-define-engines-abc
Choose a base branch
from

Conversation

shaneahmed
Copy link
Member

  • Define SemanticSegmentor with the New EngineABC

@shaneahmed shaneahmed self-assigned this Sep 20, 2024
@shaneahmed shaneahmed added the enhancement New feature or request label Sep 20, 2024
@shaneahmed shaneahmed added this to the Release v2.0.0 milestone Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.58%. Comparing base (34b3204) to head (ab4e93b).

Files with missing lines Patch % Lines
tiatoolbox/models/engine/semantic_segmentor_new.py 94.73% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           dev-define-engines-abc     #866      +/-   ##
==========================================================
+ Coverage                   91.52%   91.58%   +0.06%     
==========================================================
  Files                          73       74       +1     
  Lines                        9061     9143      +82     
  Branches                     1188     1201      +13     
==========================================================
+ Hits                         8293     8374      +81     
  Misses                        756      756              
- Partials                       12       13       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@shaneahmed shaneahmed marked this pull request as draft February 5, 2025 16:27
- Use `input_resolutions` instead of resolution to make engines outputs compatible with ioconfig.
- Uses input resolution as a list of dictionaries on units and resolution.
- Use `input_resolutions` instead of resolution to make engines outputs compatible with ioconfig.
- Uses input resolution as a list of dictionaries on units and resolution.
…mentor

# Conflicts:
#	tests/engines/test_engine_abc.py
#	tests/engines/test_patch_predictor.py
#	tiatoolbox/models/engine/engine_abc.py
#	tiatoolbox/models/engine/io_config.py
#	tiatoolbox/models/engine/patch_predictor.py
)
for layer_ in contours:
coords = layer_.squeeze()
count += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this method treats all contours as outer contours, including those that are actually holes. So it may accidentally draw holes as normal filled polygons unless further post-processing is added to distinguish parent and child contours.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will especially be a problem for semantic segmentation.

patch_output: dict | zarr.group,
scale_factor: tuple[float, float],
class_dict: dict | None = None,
save_path: Path | None = None,
) -> AnnotationStore | Path:
"""Converts (and optionally saves) output of TIAToolbox engines as AnnotationStore.
"""Converts output of TIAToolbox PatchPredictor engines to AnnotationStore.
Copy link
Contributor

Choose a reason for hiding this comment

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

engine not engines


Args:
images (list, ndarray):
List of inputs to process. when using `patch` mode, the
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalise when

@@ -572,6 +573,10 @@ class PatchDataset(PatchDatasetABC):
labels (list):
List of labels for sample at the same index in `inputs`.
Default is `None`.
patch_input_shape (tuple):
Size of patches input to the model. Patches are at
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the text be pushed one tab back to line up with labels?

Copy link
Contributor

@adamshephard adamshephard left a comment

Choose a reason for hiding this comment

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

Some minor comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants