Skip to content
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

Implement basic label matching #2

Merged
merged 25 commits into from
Nov 4, 2021
Merged

Conversation

jo-mueller
Copy link
Contributor

I implemented functions to match labels based on intersection over union (cpu) that work in 2D and 3D based on cellpose (source)

The functions are biapolutils.label.match_labels and match_labels_stack that rely on label.label_overlap_matrix and label.intersection_over_union.

A basic notebook for label-matching is available.

future versions of these functions should include

  • a verbose option to track spent time
  • a GPU accelerated version of IoU calculation. Currently it's relatively slow (~30s for 400x400x50 with ~20 labels) but it works.

for functions see biapolutils.label.match_labels and match_labels_stack
Copy link
Member

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

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

Hi Johannes @jo-mueller ,

great to see this evolving! I just put some comments in the code, mostly asking for better variable naming and some API changes.

Let me know if I can help.

Best,
Robert

def intersection_over_union(masks_true, masks_pred):
""" intersection over union of all mask pairs

From: https://github.com/MouseLand/cellpose/blob/6fddd4da98219195a2d71041fb0e47cc69a4b3a6/cellpose/metrics.py#L165
Copy link
Member

Choose a reason for hiding this comment

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

Can you please make a folder "/license_thirdparty" and put the cellpose license in there? Furthermore, a acknowledgement statment in the readme would be kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in dee6ee3 and 96e10c6


import numpy as np

def label_overlap_matrix(x, y):
Copy link
Member

Choose a reason for hiding this comment

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

Can you please rename the variables x and y? I suggest something more descriptive like label_image_x and label_image_y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in ab725ce

from skimage import io
import os

def match_labels_stack(stack, method='iou', **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

A method should not be passed as string. Please pass the method as method. See this notebook for an example. https://github.com/BiAPoL/Bio-image_Analysis_with_Python/blob/main/python_basics/12_functional_parameters.ipynb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 92167c9 - Thanks for the source!


return stack

def match_labels(imageA, imageB, method='iou', **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

please rename imageA and imageB to something more descriptive. Please also be consistent with the naming above, e.g. label_image_x

Copy link
Member

Choose a reason for hiding this comment

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

Also don't pass a method as string like explained above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Optional arguments changed in 92167c9

import tqdm
from ._intersection_over_union import intersection_over_union

def stitch3Dto3D(masks, method='iou', **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Please rename masks and pass a method, See comments above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sticht3Dto3D was removed in 31c66f1 - the function is covered by match_labels_stack and works in 2D->3D and 3D->4D.

import matplotlib.pyplot as plt

if __name__=='__main__':
path = r'D:\Documents\Promotion\Projects\2021_napari_quantify_segmentation\data\set1_unmatched'
Copy link
Member

@haesleinhuepf haesleinhuepf Nov 4, 2021

Choose a reason for hiding this comment

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

Please replace absolute paths on your computer with relative paths so that this works on all computers. Also please upload the example data. If the dataset is big, I suggest to use a smaller one. I'm personally big fan of example images with size 5x5 pixels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a data submodule with example images that are shipped with biapolutils (ba3c4fb). I couldn't find a license for blobs.gif from ImageJ (source: https://imagej.nih.gov/ij/images/)

Copy link
Member

Choose a reason for hiding this comment

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

blobs is part of ImageJ und thus, public domain :-)
https://imagej.nih.gov/ij/disclaimer.html

setup.cfg Outdated
@@ -31,4 +31,5 @@ install_requires =
numpy
scikit-image
scikit-learn
tqdm
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can see, tqdm is not used in your code. Do we really need this dependency? Please also check the other dependencies and reduce them to the absolute minimum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 3b036f4. I do think, however, that some sort of progress feedback could be valuable in some functions.

@haesleinhuepf
Copy link
Member

haesleinhuepf commented Nov 4, 2021

future versions of these functions should include

Also, could you please make individual github issues out of that list? We might otherwise forget to implement that stuff.

Thanks!

@haesleinhuepf
Copy link
Member

haesleinhuepf commented Nov 4, 2021

Some more suggestions:

  • can the main package be called "biapol_utilities"? that would match better to the repository name
  • remove version from sub-packages such as here
  • There are manifold versions of the Institute name. Please synchronize to DFG Cluster of Excellence "Physics of Life", TU Dresden.
  • There are manifold version of the Group name. Please syncronize to Bio-image Analysis Technology Development Group at DFG Cluster of Excellence "Physics of Life", TU Dresden
  • Remove boiler plate comments from files. E.g. authors here: https://github.com/BiAPoL/biapol-utilities/blob/c2bbfb41e8df894f0be0dbb4d59c50e7d362b752/biapolutils/label/__init__.py Github nicely takes care of who wrote what

@@ -0,0 +1,21 @@
"""Biapol utilities for Python
``biapol-utilities`` (a.k.a. ``biapolutils``) is a collection of utility functions
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``biapol-utilities`` (a.k.a. ``biapolutils``) is a collection of utility functions
``biapol-utilities`` (a.k.a. ``biapol_utilitiess``) is a collection of utility functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 35303e1

Label handling and evaluation
surface
Manage surfaces and point representations
util
Copy link
Member

Choose a reason for hiding this comment

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

Can we please write things out, e.g. "utilities" in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in df73737

import matplotlib.pyplot as plt

if __name__=='__main__':
path = r'D:\Documents\Promotion\Projects\2021_napari_quantify_segmentation\data\set1_unmatched'
Copy link
Member

Choose a reason for hiding this comment

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

blobs is part of ImageJ und thus, public domain :-)
https://imagej.nih.gov/ij/disclaimer.html

@jo-mueller jo-mueller merged commit dfd8eac into main Nov 4, 2021
@jo-mueller jo-mueller deleted the implement-basic-label-matching branch November 4, 2021 13:18
@haesleinhuepf
Copy link
Member

Hey Johannes @jo-mueller ,

for the future: You should not merge your own pull-request without proper review. Let others be part of the process ;-)

Best,
Robert

@haesleinhuepf haesleinhuepf mentioned this pull request Nov 4, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants