Refactor `segmentation_to_objects` #304

quantumjot · 2023-04-19T08:37:11Z

This PR refactors a critical function segmentation_to_objects:

cleans up code to remove duplications, adds modern typing
adds an (optional) progress bar if tqdm is installed, to provide feedback
add support for a multiprocessing Pool for more performant detection
allows user defined image analysis functions by exposing extra_properties
updates the example notebook to demonstrate some of the changes
a few small changes to documentation
adds some tests for the new functionality

quantumjot · 2023-04-19T08:43:11Z

These changes are to enable @ania-m-b to do some "advanced" analysis.

deprecated-napari-hub-preview-bot · 2023-04-19T08:44:23Z

Preview page for your plugin is ready here:
https://preview.napari-hub.org/quantumjot/btrack/304
Updated: 2023-04-20T11:30:19.635085

quantumjot · 2023-04-20T09:57:26Z

@paddyroddy and @p-j-smith - this should also help with some aspects of running this from the napari plugin

paddyroddy

Have added some thoughts

paddyroddy · 2023-04-20T10:31:58Z

tests/test_utils.py

+    def extra_prop(_mask) -> float:
+        return np.sum(_mask)
+
+    extra_properties = (extra_prop,)


Why not just something like this?

Suggested change

extra_properties = (extra_prop,)

extra_properties = (np.sum(_mask),)

Happy to do it this way, but I was trying to emulate how an end user would actually use this. Probably doesn't matter in the tests

Ah I see. I couldn't really make sense of it.

The idea is that you can now provide functions that work on the image data and return properties that become part of the PyTrackObject properties - it's much cleaner to use and makes end-use much more powerful

paddyroddy · 2023-04-20T10:40:12Z

btrack/io/_localization.py

+    # this provides a dummy progress bar in case `tqdm` is not installed.
+    def tqdm(iterator, *args, **kwargs):
+        logger.info("Try installing ``tqdm`` for progress bar rendering.")
+        return iterator


Why wouldn't tqdm be installed?

I just left it optional, we could add to the requirements

I don't see any harm in that?

If we want it optional could add something similar to optional-dependencies.docs

I've added it as a requirement. I think it could be useful in other parts of the codebase too

paddyroddy · 2023-04-20T10:42:01Z

btrack/io/_localization.py

+    def _next_array(self) -> Tuple[npt.NDArray, Optional[npt.NDArray]]:
+        """__next__ method for an array-like input."""
+        if self._iter >= len(self):
+            raise StopIteration


What happens when this is raised? Should we have some sort of logging?

This StopIteration just allows the iterator to stop when it get's to the end of the data. It doesn't interrupt execution of the code

paddyroddy · 2023-04-20T10:43:46Z

btrack/io/_localization.py

+    centroid_type: str = "centroid"
+    intensity_image: Optional[npt.NDArray] = None
+    scale: Optional[Tuple[float]] = None
+    assign_class_ID: bool = False  # noqa: N815


Once python=3.10 is the min we support, we can use dataclasses.KW_ONLY to avoid the noqa here

Nice! I think this is actually because of the mixed case here

Oops, I should have checked what the error is for

paddyroddy · 2023-04-20T10:46:44Z

btrack/io/_localization.py

 import logging
-from typing import Generator, List, Optional, Tuple, Union
+from multiprocessing.pool import ThreadPool as Pool


Have you considered concurrent.futures.ThreadPoolExecutor?

Oops - this is a good catch, it's actually supposed to be just a Pool

I feel like this is something we can take a look at when this is integrated into the napari plugin - I'm not sure what the best way to play with that is yet

btrack/io/_localization.py

Co-authored-by: Patrick Roddy <patrickjamesroddy@gmail.com>

codecov-commenter · 2023-04-20T11:10:22Z

Codecov Report

Patch coverage: 95.74% and project coverage change: +0.37 🎉

Comparison is base (146419f) 84.09% compared to head (07721e5) 84.47%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #304      +/-   ##
==========================================
+ Coverage   84.09%   84.47%   +0.37%     
==========================================
  Files          30       30              
  Lines        1905     1932      +27     
  Branches      295      295              
==========================================
+ Hits         1602     1632      +30     
+ Misses        216      215       -1     
+ Partials       87       85       -2

Impacted Files	Coverage Δ
btrack/io/_localization.py	`91.89% <95.74%> (+6.17%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

quantumjot · 2023-04-20T11:39:49Z

Thanks for the review @paddyroddy - I think I took care of everything.

quantumjot added 7 commits April 18, 2023 18:24

refactor segmentation_to_objects

2a56db7

add test

b5d2374

add processing pool

d731f9c

remove comments

3b75a0f

add docstring

8e73fef

update notebooks

25e9ac7

update docstrings

4ffd0df

quantumjot marked this pull request as ready for review April 20, 2023 08:50

quantumjot requested a review from paddyroddy April 20, 2023 09:57

paddyroddy reviewed Apr 20, 2023

View reviewed changes

quantumjot and others added 3 commits April 20, 2023 12:01

Update btrack/io/_localization.py

6d3c1a3

Co-authored-by: Patrick Roddy <patrickjamesroddy@gmail.com>

Update btrack/io/_localization.py

7336ce0

Co-authored-by: Patrick Roddy <patrickjamesroddy@gmail.com>

Update btrack/io/_localization.py

6e66000

Co-authored-by: Patrick Roddy <patrickjamesroddy@gmail.com>

quantumjot added 4 commits April 20, 2023 12:21

update requirements to add tqdm

d10f007

apply suggestions

8cf1b58

reduce number of tests

322e368

update notebook

07721e5

quantumjot requested a review from paddyroddy April 20, 2023 11:39

quantumjot merged commit 69fc3a8 into main Apr 20, 2023

quantumjot deleted the regionprops-extra branch April 20, 2023 11:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `segmentation_to_objects` #304

Refactor `segmentation_to_objects` #304

quantumjot commented Apr 19, 2023 •

edited

Loading

quantumjot commented Apr 19, 2023

deprecated-napari-hub-preview-bot bot commented Apr 19, 2023 •

edited

Loading

quantumjot commented Apr 20, 2023

paddyroddy left a comment

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

paddyroddy Apr 20, 2023

paddyroddy Apr 20, 2023

quantumjot Apr 20, 2023

quantumjot Apr 20, 2023

codecov-commenter commented Apr 20, 2023 •

edited

Loading

quantumjot commented Apr 20, 2023

	extra_properties = (extra_prop,)
	extra_properties = (np.sum(_mask),)

Refactor segmentation_to_objects #304

Refactor segmentation_to_objects #304

Conversation

quantumjot commented Apr 19, 2023 • edited Loading

quantumjot commented Apr 19, 2023

deprecated-napari-hub-preview-bot bot commented Apr 19, 2023 • edited Loading

quantumjot commented Apr 20, 2023

paddyroddy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 20, 2023 • edited Loading

Codecov Report

quantumjot commented Apr 20, 2023

Refactor `segmentation_to_objects` #304

Refactor `segmentation_to_objects` #304

quantumjot commented Apr 19, 2023 •

edited

Loading

deprecated-napari-hub-preview-bot bot commented Apr 19, 2023 •

edited

Loading

codecov-commenter commented Apr 20, 2023 •

edited

Loading