Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #309 +/- ##
==========================================
- Coverage 83.73% 83.36% -0.37%
==========================================
Files 8 8
Lines 1543 1653 +110
==========================================
+ Hits 1292 1378 +86
- Misses 251 275 +24
|
src/spatialdata_plot/pl/render.py
Outdated
| trans = mtransforms.Affine2D(matrix=affine_trans) | ||
| trans_data = trans + ax.transData | ||
|
|
||
| rgba_image = np.transpose(rgba_image.data.compute(), (1, 2, 0)) # type: ignore[attr-defined] |
There was a problem hiding this comment.
Here (and in render_shapes), we access the image as numpy array from the SpatialImage. mypy doesn't believe that compute() exists...
src/spatialdata_plot/pl/render.py
Outdated
|
|
||
| # compute canvas size in pixels close to the actual image size to speed up computation | ||
| plot_width = x_ext[1] - x_ext[0] | ||
| plot_height = y_ext[1] - y_ext[0] | ||
| plot_width_px = int(round(fig_params.fig.get_size_inches()[0] * fig_params.fig.dpi)) | ||
| plot_height_px = int(round(fig_params.fig.get_size_inches()[1] * fig_params.fig.dpi)) | ||
| factor = np.min([plot_width / plot_width_px, plot_height / plot_height_px]) | ||
| plot_width = int(np.round(plot_width / factor)) | ||
| plot_height = int(np.round(plot_height / factor)) |
There was a problem hiding this comment.
I'd consider bundling this code in a private function since it's duplicate from above.
src/spatialdata_plot/pl/render.py
Outdated
|
|
||
| rbga_image = np.transpose(ds_result.to_numpy().base, (0, 1, 2)) | ||
| cax = ax.imshow(rbga_image, zorder=render_params.zorder, alpha=render_params.alpha) | ||
| # create SpatialImage to get it back to original size |
There was a problem hiding this comment.
Also this code is a duplicate, I'd consider it refactoring it into a private function.
…f eq_hist for ds.shade()
|
Thanks @Sonja-Stockhaus ! I have tried the PR on the Visium HD data and the speed up is significant! I switched back to using |
|
Sonja, I found a bug with this PR when the rasterized object needs to be aligned with the rest #291. The solution seems straightforward, I think one just needs to initialize the image element from datashader using the old coordinate transformations. |
|
Will look into it! |
|
Thanks for the update!
This task here seems tricky. I would consider raising a warning saying that the outline is currently not supported and workin on this on a separate PR (with low priority). |
If you use |
|
Ah ok, then it should be a fast addition, thanks for the update. |
|
General problem of datashader so far: when you render elements, coloring them by a value and a color map, you wouldn't see a single element if all of them have the same value Fix in this PR: internally, not the whole colormap is passed to datashader, but just the color given by |
|
Thanks for the explanation, I think the approach that you implemented is a good one! |
melonora
left a comment
melonora
left a comment
There was a problem hiding this comment.
Hi @Sonja-Stockhaus, thanks for the amazing work. I think it is very far already, but would like some minor changes here and there, mainly to shorten the code a bit and some refactorings to keep it more maintainable.
|
As discussed with @timtreis, we now use |
5 participants
This speeds everything up for me locally, but for the benchmark (#296), I see an effect (aka datashader faster than matplotlib) for e.g. 10k points/shapes etc.