-
Notifications
You must be signed in to change notification settings - Fork 98
Add helper functions to tile image for inference #1053
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
Conversation
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1053 +/- ##
==========================================
+ Coverage 65.52% 65.88% +0.35%
==========================================
Files 43 44 +1
Lines 6364 6762 +398
Branches 1064 1137 +73
==========================================
+ Hits 4170 4455 +285
- Misses 1810 1874 +64
- Partials 384 433 +49
🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
Zethson
left a comment
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.
A few random comments as requested
for more information, see https://pre-commit.ci
| preview: bool = True, | ||
| ) -> None: | ||
| """ | ||
| Create tiles centered on Visium spots, classify them by tissue coverage, and optionally render a preview. |
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.
Is it necessary to restrict this to Visium spots? This could also work for arbitrary segmentations as long as cell coordinates + e.g radius are given. It definitely works on custom cell circles.
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 think you need to import spatialdata-plot to fix that
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.
Is it necessary to restrict this to Visium spots? This could also work for arbitrary segmentations
I think yes, because it generates non-overlapping tiles. Yes, you can generate them centered around cell segmentation masks but then the tiles would overlap which would probably create interpretability issues. So I'd semantically restrict it to the clean usecase - what users then actually do with it is their issue imho.
I think you need to import spatialdata-plot to fix that
wdym @selmanozleyen ?
LLehner
left a comment
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.
Looks good! Just wondering about the plot warning, but maybe thats on my side.
selmanozleyen
left a comment
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.
Besides my comments I checked the code coverage and it suggested that some cases weren't tested.
-
if H_mask != H or W_mask != W: # (line 803)
I am not sure if its accurate but overall I'd check the coverage report to ensure robustness for some cases.
for more information, see https://pre-commit.ci
selmanozleyen
left a comment
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.
looks good
Uh oh!
There was an error while loading. Please reload this page.