-
Notifications
You must be signed in to change notification settings - Fork 98
Method to detect specimens in H&E images #1044
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
base: main
Are you sure you want to change the base?
Method to detect specimens in H&E images #1044
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi these are some initial feedbacks I will get into more details tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd actually prefer calling this module experimental
instead of exp
(also similar to {anndata,scanpy}.experimental
module). Because I was wondering what it was until I saw this.
If the mask is saved to the SpatialData object, it will inherit the scale_factors | ||
of the image, if present. | ||
**kwargs | ||
Optional keyword arguments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am strongly against using kwargs. I can easily imagine the user mistyping felzenszwalb_params
and wondering why it doesn't change the results :D . We need errors for cases like these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, makes no sense to arbitrarily nest them in the docs and collect them into a dict in the code.
I think having advanced parameters tucked away in the data class signatures the way it is is ideal already.
“Optional keyword arguments” are keyword arguments that have a default, so often all of them.
sdata: sd.SpatialData, | ||
image_key: str, | ||
scale: str = "auto", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always use a *
if a function signature that has more than like 2 or 3 arguments total. Usually you can just put it after the parameters that have no default, in some cases it makes sense that the first optional parameter can be specified by keyword or position.
sdata: sd.SpatialData, | |
image_key: str, | |
scale: str = "auto", | |
sdata: sd.SpatialData, | |
image_key: str, | |
*, | |
scale: str = "auto", |
I’d personally do this to make sdata
a mandatory positional parameter, but people can still do image_key="..."
if they want:
sdata: sd.SpatialData, | |
image_key: str, | |
scale: str = "auto", | |
sdata: sd.SpatialData, | |
/, | |
image_key: str, | |
*, | |
scale: str = "auto", |
For other downstream functions, such as #1036, one needs a function to robustly detect where in the image the tissue is and how many of those there are.
This function
a) implements two algorithms for identifying the tissue (otsu & felzenszwalb)
b) deals with arbitrary channel input
c) heuristically tries to identify what is a sample and what is either just random stuff (dirt, Visium frame etc). As a fallback, one can pass in the number of samples expected which should be more robust
d) adds the mask back to the sdata object with the same structure and transformations as the original image had
e) does everything in dask so it's quite fast
Todo