Skip to content

MVP for morpohology module #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.

Sign up for GitHub

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

Closed
wants to merge 25 commits into from
Closed

MVP for morpohology module #866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b99fa5f
MVP for internal calling of extra props
timtreis Aug 7, 2024
bbecec3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 7, 2024
6e3d652
Added option to externally feed in functions
timtreis Aug 8, 2024
7b21fda
merge conflict resolved
timtreis Aug 8, 2024
753ec3a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 8, 2024
c9a3cc9
Merge branch 'main' into feature/add_morphology_toolbox
timtreis Aug 15, 2024
34f745f
add rough functionality to write regionprops into sdata["table"].obsm…
npeschke Aug 16, 2024
07606e2
DataFrame now written to obsm instead of numpy array
npeschke Aug 23, 2024
0f0ea7c
add granularity measurement from afermg/cp_measurements
npeschke Aug 27, 2024
b33895b
add border_occupied_factor
npeschke Aug 29, 2024
ebc08d8
add sanity checks and make multiple coordinate systems work
npeschke Aug 29, 2024
50d58d2
fix assertion to accommodate new interface
npeschke Aug 29, 2024
328ba16
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 29, 2024
f7b5270
add possibility for granularity to return multiple values per channel…
npeschke Sep 3, 2024
5c1d70f
Merge remote-tracking branch 'origin/feature/add_morphology_toolbox' …
npeschke Sep 3, 2024
112d1a9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 3, 2024
0ba44d2
Merge branch 'main' into feature/add_morphology_toolbox
npeschke Sep 3, 2024
8047dde
add "all" option for regionprops through using methods=None
npeschke Sep 5, 2024
39f9db8
add granularity and border_occupied_factor to _all_regionprops_names
npeschke Sep 24, 2024
02d21c8
refactor quantify_morphology
npeschke Oct 8, 2024
59d8137
add zernike
npeschke Oct 8, 2024
bc035d2
status update
npeschke Nov 14, 2024
cde0526
handle dict returns from calculate_* features
npeschke Nov 22, 2024
9a9511f
add _sdata_image_features_helper
npeschke Jan 21, 2025
78c0f67
Merge branch 'main' into feature/add_morphology_toolbox
timtreis Mar 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ dependencies = [
"tifffile!=2022.4.22",
"tqdm>=4.50.2",
"validators>=0.18.2",
"centrosome>=1.2.3",
"cp-measure>=0.1.4"
"xarray>=2024.10.0",
"zarr>=2.6.1,<3.0.0",
"spatialdata>=0.2.5",
Expand Down
2 changes: 1 addition & 1 deletion src/squidpy/im/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from __future__ import annotations

from squidpy.im._container import ImageContainer
from squidpy.im._feature import calculate_image_features
from squidpy.im._feature import calculate_image_features, quantify_morphology
Copy link
Member

Choose a reason for hiding this comment

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

hi @npeschke @timtreis was just taking a look at this, was wondering why the duplication of the function? It seems that quantify_morphology does something very similar to calculate_image_features. Do you plan to discontinue the latter? wouldn't it be better to adapt the latter to use spatialdata + adapt? thank you

Copy link
Member Author

Choose a reason for hiding this comment

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

tbh not sure yet what the optimal solution will be. There is clear redundancy and a strong overlap, but when I wrote the original MVP, it didn't clearly fit with the current calculate_image_features. On the one hand, some of these features don't need an image but only the label, but then also the structure of the function was going to be quite different.

Even if we merge them into the calculate_image_features function, the parameters of that function would have to change. So yeah, not fully sure yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

I generally wanted to have sth that takes an arbitrary callable with a region_props like footprint so we could also inject f.e. some HugginfFace featuriser or sth

Copy link
Member

Choose a reason for hiding this comment

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

right, that's a good point. The calculate_image_features does support arbitrary functions, see https://squidpy.readthedocs.io/en/stable/api/squidpy.im.calculate_image_features.html and https://squidpy.readthedocs.io/en/stable/api/squidpy.im.calculate_image_features.html and I understand that the current implementation doesn't operate on masks (or not only on mask), but maybe it would be ok to just change the input to "image" to not be just an Image but also a Label?

Copy link
Member

Choose a reason for hiding this comment

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

the parallelization/out of core functionality I think it's also important. The current implementation relies on joblib parallel to iterate over spots, and understand it's not optimal (e.g. how to do it with raster labels and raster image?). But maybe a combination of that and dask, e.g. https://examples.dask.org/applications/image-processing.html could be useful. Basically I think we should strive to implement scalability in time/memory.

Copy link
Member

@giovp giovp Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

looking at this, not maintained but maybe some ideas could be reused
https://github.com/jrussell25/dask-regionprops/blob/main/dask_regionprops/regionprops.py

EDIT: looking more deeply, not sure anymore 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this ties into the larger topic of moving things to the GPU

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for context, the current implementation takes 20 seconds on my machine to calculate all available features of the MIBI-TOF dataset. So parallelization might not need to be that urgent.

Copy link
Member Author

@timtreis timtreis Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It'll be if we add more, potentially more expensive to compute, features and analyse datasets like Xenium with 100k+ cells ;)

Copy link
Member

Choose a reason for hiding this comment

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

hi @npeschke , thanks for sharing the time. If the mibi-tof dataset you are referring to is the one from squidpy, that is a toy dataset that does not really recapitulate real data complexity and size. I'd be curious to see the performance on a e.g. xenium dataset.

from squidpy.im._process import process
from squidpy.im._segment import (
SegmentationCustom,
Expand Down
Loading
Loading