illustration notebook for rasterization & multiscale images #62
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Hi, nice notebook! Please let me know if it's ready for review 😊 |
|
Hi @LucaMarconato! Good point, I kind of forgot about this PR, thanks for bringing it up! It's done and ready for review. |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:42Z Give a header like "Implicit performance improvements while rendering" or sth? Also explain clearer that these steps are happening automatically under the hood but you'll explain how one can take control over it. Good that you provide the download link! |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:43Z Line #2. sdata = sd.read_zarr("D:/hiwi/visium_associated_xenium_io.zarr")
Replace path with "./visium_associated_xenium_io.zarr" |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:44Z Add a tiny explanation what multi-scale images are and why we have to differentiate behaviour here |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:45Z Line #1. %%time good! But rerurn the entire notebook suppressing warnings |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:46Z Phrase more clearly |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:46Z Explain the influence |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:47Z Is it acutally not rasterizing here? Or is the "downscaled_hires" just small? Maybe illustrate with the largest image in the dataset Sonja-Stockhaus commented on 2024-01-10T14:54:36Z The image is just too small to see a large effect. There is no larger single-scale image, but I illustrate it with the largest image in the multi-scale image (very last image in the notebook). That just doesn't belong in the single-scale section imo
|
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:48Z nitpicky, but I think it's based on the size of the rendering device? |
|
View / edit / reply to this conversation on ReviewNB timtreis commented on 2024-01-09T20:28:49Z Line #3. # "scale4" is automatically selected What is your opinion on adding this as a INFO to the logger? Sonja-Stockhaus commented on 2024-01-10T14:10:54Z I originally included the info logging but then removed it after Luca's suggestion (scverse/spatialdata-plot#164 (comment) I found it helpful during development/debugging and maybe won't hurt in daily use, just not sure how many users actually want to know that (especially when none of the arguments is set). If you find it important I'm open to include it again :) I just added the comment here to demonstrate what's happening under the hood timtreis commented on 2024-01-10T14:20:46Z Okay, fair point! |
|
I originally included the info logging but then removed it after Luca's suggestion (scverse/spatialdata-plot#164 (comment) I found it helpful during development/debugging and maybe won't hurt in daily use, just not sure how many users actually want to know that (especially when none of the arguments is set). If you find it important I'm open to include it again :) I just added the comment here to demonstrate what's happening under the hood View entire conversation on ReviewNB |
|
Okay!
View entire conversation on ReviewNB |
|
The image is just too small to see a large effect. There is no larger single-scale image, but I illustrate it with the largest image in the multi-scale image (very last image in the notebook). That just doesn't belong in the single-scale section imo
View entire conversation on ReviewNB |
|
Thanks for the feedback @timtreis! I updated the notebook including most of your input |
|
Very nice and clear, good job! Pre-approving only with one minor comment. I guess that dpi and figsize can be specified also before calling the plotting function (e.g. I would show this as an extra comment to the notebook since it would be useful in scenarios like having multiple subplots manually created by the user. |
|
Thanks for pointing it out @LucaMarconato, it's now included |
|
@LucaMarconato Do you have any idea why the build fails? It's not modifying the toml |
|
I also had some weird problems with the docs today, I was getting a different unexpected error message and simply manually retriggering the build made the docs pass. Let's see if it works also for this PR. |
|
No luck, maybe it's a problem with dependencies (the error message looks like the one here scverse/napari-spatialdata#78). |
for more information, see https://pre-commit.ci
…/spatialdata-notebooks into feature/59-speed-up-illustration
|
Fixed it by adjusting the headline levels in @Sonja-Stockhaus notebook, added it to the menu and also added the following change to the .toml because the warning told me to |
No description provided.