From ba46c35682a73d49c03b0065147917d4f4784dc1 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:37:08 +0100 Subject: [PATCH 1/6] simplify affine matrix calculation for datashader --- README.md | 4 +- docs/contributing.md | 10 ++--- src/spatialdata_plot/pl/render.py | 8 ++-- src/spatialdata_plot/pl/utils.py | 64 +++++++++++++++---------------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index ba786da3..5dc823a5 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,8 @@ SpatialData’s plotting capabilities allow to quickly visualise all contained m For more information on the `spatialdata-plot` library, please refer to the [documentation](https://spatialdata.scverse.org/projects/plot/en/latest/index.html). In particular, the -- [API documentation][link-api]. -- [Example notebooks][link-notebooks] (section "Visiualizations") +- [API documentation][link-api]. +- [Example notebooks][link-notebooks] (section "Visiualizations") ## Installation diff --git a/docs/contributing.md b/docs/contributing.md index 10d0bd8b..e2361673 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -6,8 +6,8 @@ Please refer to the [contribution guide from the `spatialdata` repository](https Many tests will produce plots and check that they are correct by comparing them with a previously saved and serialized version of the same plots. The ground truth images are located in `tests/_images`. Different OS/versions may produce similar but not identical plots (for instance the ticks/padding could vary). To take into account for this please consider the following: -- you should not use locally generated plots as ground truth images, but you should commit images that have been generated by a GitHub Action. The recommended workflow is to go to the ["actions" page for the repo](https://github.com/scverse/spatialdata-plot/actions/workflows/test.yaml), download the artifacts, and upload them as ground truth (after having reviewed them). -- the ground truth images need to be updated when a new test is passing, or when a test starts producing a slightly different (but consistent) plot. -- please never replace the ground truth images without having manually reviewed them. -- if you run the tests locally in macOS or Windows they will likely fail because the ground truth images are generated using Ubuntu. To overcome this you can use `act`, which will generate a Docker reproducing the environment used in the GitHub Action. After the Docker container is generated you can use it within IDEs to run tests and debug code. -- in the case of PyCharm, it is easier to create a container from a `Dockerfile` instead of using `act`. Please in such case use the `Dockerfile` made availabel in the repository. If you encountering problems with `act` or `docker`, please [get in touch with the developers via Zulip](https://scverse.zulipchat.com/#narrow/channel/443514-spatialdata-dev) and we will help troubleshoot the issue. See also additional details [here](https://github.com/scverse/spatialdata-plot/pull/397). +- you should not use locally generated plots as ground truth images, but you should commit images that have been generated by a GitHub Action. The recommended workflow is to go to the ["actions" page for the repo](https://github.com/scverse/spatialdata-plot/actions/workflows/test.yaml), download the artifacts, and upload them as ground truth (after having reviewed them). +- the ground truth images need to be updated when a new test is passing, or when a test starts producing a slightly different (but consistent) plot. +- please never replace the ground truth images without having manually reviewed them. +- if you run the tests locally in macOS or Windows they will likely fail because the ground truth images are generated using Ubuntu. To overcome this you can use `act`, which will generate a Docker reproducing the environment used in the GitHub Action. After the Docker container is generated you can use it within IDEs to run tests and debug code. +- in the case of PyCharm, it is easier to create a container from a `Dockerfile` instead of using `act`. Please in such case use the `Dockerfile` made availabel in the repository. If you encountering problems with `act` or `docker`, please [get in touch with the developers via Zulip](https://scverse.zulipchat.com/#narrow/channel/443514-spatialdata-dev) and we will help troubleshoot the issue. See also additional details [here](https://github.com/scverse/spatialdata-plot/pull/397). diff --git a/src/spatialdata_plot/pl/render.py b/src/spatialdata_plot/pl/render.py index 5bafe7a8..d44b90d5 100644 --- a/src/spatialdata_plot/pl/render.py +++ b/src/spatialdata_plot/pl/render.py @@ -19,7 +19,7 @@ from scanpy._settings import settings as sc_settings from spatialdata import get_extent, get_values, join_spatialelement_table from spatialdata.models import PointsModel, ShapesModel, get_table_keys -from spatialdata.transformations import get_transformation, set_transformation +from spatialdata.transformations import set_transformation from spatialdata.transformations.transformations import Identity from xarray import DataTree @@ -43,7 +43,6 @@ _get_colors_for_categorical_obs, _get_extent_and_range_for_datashader_canvas, _get_linear_colormap, - _get_transformation_matrix_for_datashader, _is_coercable_to_float, _map_color_seg, _maybe_set_colors, @@ -184,10 +183,9 @@ def _render_shapes( sdata_filt.shapes[element].loc[is_point, "geometry"] = _geometry[is_point].buffer(scale.to_numpy()) # apply transformations to the individual points - element_trans = get_transformation(sdata_filt.shapes[element]) - tm = _get_transformation_matrix_for_datashader(element_trans) + tm = trans.get_matrix() transformed_element = sdata_filt.shapes[element].transform( - lambda x: (np.hstack([x, np.ones((x.shape[0], 1))]) @ tm)[:, :2] + lambda x: (np.hstack([x, np.ones((x.shape[0], 1))]) @ tm.T)[:, :2] ) transformed_element = ShapesModel.parse( gpd.GeoDataFrame(data=sdata_filt.shapes[element].drop("geometry", axis=1), geometry=transformed_element) diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 03941dd8..625320b3 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -19,7 +19,6 @@ import matplotlib.transforms as mtransforms import numpy as np import numpy.ma as ma -import numpy.typing as npt import pandas as pd import shapely import spatialdata as sd @@ -61,8 +60,7 @@ from spatialdata.models import Image2DModel, Labels2DModel, SpatialElement # from spatialdata.transformations.transformations import Scale -from spatialdata.transformations import Affine, Identity, MapAxis, Scale, Translation -from spatialdata.transformations import Sequence as SDSequence +from spatialdata.transformations import Scale from spatialdata.transformations.operations import get_transformation from xarray import DataArray, DataTree @@ -2122,6 +2120,8 @@ def _create_image_from_datashader_result( # create SpatialImage from datashader output to get it back to original size rgba_image_data = ds_result.to_numpy().base rgba_image_data = np.transpose(rgba_image_data, (2, 0, 1)) + # # flip y axis data since datashader renders assuming that the origin is at the top left corner + # rgba_image_data = rgba_image_data[:, ::-1, :] rgba_image = Image2DModel.parse( rgba_image_data, dims=("c", "y", "x"), @@ -2237,32 +2237,32 @@ def _prepare_transformation( return trans, trans_data -def _get_datashader_trans_matrix_of_single_element( - trans: Identity | Scale | Affine | MapAxis | Translation, -) -> npt.NDArray[Any]: - flip_matrix = np.array([[1, 0, 0], [0, -1, 0], [0, 0, 1]]) - tm: npt.NDArray[Any] = trans.to_affine_matrix(("x", "y"), ("x", "y")) - - if isinstance(trans, Identity): - return np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) - if isinstance(trans, (Scale | Affine)): - # idea: "flip the y-axis", apply transformation, flip back - flip_and_transform: npt.NDArray[Any] = flip_matrix @ tm @ flip_matrix - return flip_and_transform - if isinstance(trans, MapAxis): - # no flipping needed - return tm - # for a Translation, we need the transposed transformation matrix - return tm.T - - -def _get_transformation_matrix_for_datashader( - trans: Scale | Identity | Affine | MapAxis | Translation | SDSequence, -) -> npt.NDArray[Any]: - """Get the affine matrix needed to transform shapes for rendering with datashader.""" - if isinstance(trans, SDSequence): - tm = np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) - for x in trans.transformations: - tm = tm @ _get_datashader_trans_matrix_of_single_element(x) - return tm - return _get_datashader_trans_matrix_of_single_element(trans) +# def _get_datashader_trans_matrix_of_single_element( +# trans: Identity | Scale | Affine | MapAxis | Translation, +# ) -> npt.NDArray[Any]: +# flip_matrix = np.array([[1, 0, 0], [0, -1, 0], [0, 0, 1]]) +# tm: npt.NDArray[Any] = trans.to_affine_matrix(("x", "y"), ("x", "y")) +# +# if isinstance(trans, Identity): +# return np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) +# if isinstance(trans, (Scale | Affine)): +# # idea: "flip the y-axis", apply transformation, flip back +# flip_and_transform: npt.NDArray[Any] = flip_matrix @ tm @ flip_matrix +# return flip_and_transform +# if isinstance(trans, MapAxis): +# # no flipping needed +# return tm +# # for a Translation, we need the transposed transformation matrix +# return tm.T +# +# +# def _get_transformation_matrix_for_datashader( +# trans: Scale | Identity | Affine | MapAxis | Translation | SDSequence, +# ) -> npt.NDArray[Any]: +# """Get the affine matrix needed to transform shapes for rendering with datashader.""" +# if isinstance(trans, SDSequence): +# tm = np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) +# for x in trans.transformations: +# tm = tm @ _get_datashader_trans_matrix_of_single_element(x) +# return tm +# return _get_datashader_trans_matrix_of_single_element(trans) From 834807d66eb25846a28e1f497f34105d3b830f2e Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:38:40 +0100 Subject: [PATCH 2/6] remove comment; fix prettier --- .pre-commit-config.yaml | 4 ++-- src/spatialdata_plot/pl/utils.py | 31 ------------------------------- 2 files changed, 2 insertions(+), 33 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b055853a..eb286b32 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,8 +12,8 @@ repos: rev: 24.10.0 hooks: - id: black - - repo: https://github.com/pre-commit/mirrors-prettier - rev: v4.0.0-alpha.8 + - repo: https://github.com/rbubley/mirrors-prettier + rev: v3.5.0 hooks: - id: prettier - repo: https://github.com/asottile/blacken-docs diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 625320b3..699b2e36 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -2235,34 +2235,3 @@ def _prepare_transformation( trans_data = trans + ax.transData if ax is not None else None return trans, trans_data - - -# def _get_datashader_trans_matrix_of_single_element( -# trans: Identity | Scale | Affine | MapAxis | Translation, -# ) -> npt.NDArray[Any]: -# flip_matrix = np.array([[1, 0, 0], [0, -1, 0], [0, 0, 1]]) -# tm: npt.NDArray[Any] = trans.to_affine_matrix(("x", "y"), ("x", "y")) -# -# if isinstance(trans, Identity): -# return np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) -# if isinstance(trans, (Scale | Affine)): -# # idea: "flip the y-axis", apply transformation, flip back -# flip_and_transform: npt.NDArray[Any] = flip_matrix @ tm @ flip_matrix -# return flip_and_transform -# if isinstance(trans, MapAxis): -# # no flipping needed -# return tm -# # for a Translation, we need the transposed transformation matrix -# return tm.T -# -# -# def _get_transformation_matrix_for_datashader( -# trans: Scale | Identity | Affine | MapAxis | Translation | SDSequence, -# ) -> npt.NDArray[Any]: -# """Get the affine matrix needed to transform shapes for rendering with datashader.""" -# if isinstance(trans, SDSequence): -# tm = np.array([[1, 0, 0], [0, 1, 0], [0, 0, 1]]) -# for x in trans.transformations: -# tm = tm @ _get_datashader_trans_matrix_of_single_element(x) -# return tm -# return _get_datashader_trans_matrix_of_single_element(trans) From 29c3f57ce8b754e5dfd5f9f5aaacb6c24dec31c7 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:39:21 +0100 Subject: [PATCH 3/6] rerun prettier --- README.md | 4 ++-- docs/contributing.md | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5dc823a5..ba786da3 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,8 @@ SpatialData’s plotting capabilities allow to quickly visualise all contained m For more information on the `spatialdata-plot` library, please refer to the [documentation](https://spatialdata.scverse.org/projects/plot/en/latest/index.html). In particular, the -- [API documentation][link-api]. -- [Example notebooks][link-notebooks] (section "Visiualizations") +- [API documentation][link-api]. +- [Example notebooks][link-notebooks] (section "Visiualizations") ## Installation diff --git a/docs/contributing.md b/docs/contributing.md index e2361673..10d0bd8b 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -6,8 +6,8 @@ Please refer to the [contribution guide from the `spatialdata` repository](https Many tests will produce plots and check that they are correct by comparing them with a previously saved and serialized version of the same plots. The ground truth images are located in `tests/_images`. Different OS/versions may produce similar but not identical plots (for instance the ticks/padding could vary). To take into account for this please consider the following: -- you should not use locally generated plots as ground truth images, but you should commit images that have been generated by a GitHub Action. The recommended workflow is to go to the ["actions" page for the repo](https://github.com/scverse/spatialdata-plot/actions/workflows/test.yaml), download the artifacts, and upload them as ground truth (after having reviewed them). -- the ground truth images need to be updated when a new test is passing, or when a test starts producing a slightly different (but consistent) plot. -- please never replace the ground truth images without having manually reviewed them. -- if you run the tests locally in macOS or Windows they will likely fail because the ground truth images are generated using Ubuntu. To overcome this you can use `act`, which will generate a Docker reproducing the environment used in the GitHub Action. After the Docker container is generated you can use it within IDEs to run tests and debug code. -- in the case of PyCharm, it is easier to create a container from a `Dockerfile` instead of using `act`. Please in such case use the `Dockerfile` made availabel in the repository. If you encountering problems with `act` or `docker`, please [get in touch with the developers via Zulip](https://scverse.zulipchat.com/#narrow/channel/443514-spatialdata-dev) and we will help troubleshoot the issue. See also additional details [here](https://github.com/scverse/spatialdata-plot/pull/397). +- you should not use locally generated plots as ground truth images, but you should commit images that have been generated by a GitHub Action. The recommended workflow is to go to the ["actions" page for the repo](https://github.com/scverse/spatialdata-plot/actions/workflows/test.yaml), download the artifacts, and upload them as ground truth (after having reviewed them). +- the ground truth images need to be updated when a new test is passing, or when a test starts producing a slightly different (but consistent) plot. +- please never replace the ground truth images without having manually reviewed them. +- if you run the tests locally in macOS or Windows they will likely fail because the ground truth images are generated using Ubuntu. To overcome this you can use `act`, which will generate a Docker reproducing the environment used in the GitHub Action. After the Docker container is generated you can use it within IDEs to run tests and debug code. +- in the case of PyCharm, it is easier to create a container from a `Dockerfile` instead of using `act`. Please in such case use the `Dockerfile` made availabel in the repository. If you encountering problems with `act` or `docker`, please [get in touch with the developers via Zulip](https://scverse.zulipchat.com/#narrow/channel/443514-spatialdata-dev) and we will help troubleshoot the issue. See also additional details [here](https://github.com/scverse/spatialdata-plot/pull/397). From ce8307de4bfb3015c703e249c491b3f707c82dbd Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:40:06 +0100 Subject: [PATCH 4/6] remove unused code --- src/spatialdata_plot/pl/utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 699b2e36..99759ac3 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -2120,8 +2120,6 @@ def _create_image_from_datashader_result( # create SpatialImage from datashader output to get it back to original size rgba_image_data = ds_result.to_numpy().base rgba_image_data = np.transpose(rgba_image_data, (2, 0, 1)) - # # flip y axis data since datashader renders assuming that the origin is at the top left corner - # rgba_image_data = rgba_image_data[:, ::-1, :] rgba_image = Image2DModel.parse( rgba_image_data, dims=("c", "y", "x"), From 8988d6cbb8d021f0b1ebc88d2b1816a10c114cbe Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:41:01 +0100 Subject: [PATCH 5/6] cleanup --- src/spatialdata_plot/pl/utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 99759ac3..f5cc3d79 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -58,10 +58,8 @@ from spatialdata._core.query.relational_query import _locate_value from spatialdata._types import ArrayLike from spatialdata.models import Image2DModel, Labels2DModel, SpatialElement - -# from spatialdata.transformations.transformations import Scale -from spatialdata.transformations import Scale from spatialdata.transformations.operations import get_transformation +from spatialdata.transformations.transformations import Scale from xarray import DataArray, DataTree from spatialdata_plot._logging import logger From 55a1c1408f0caff2e28da125e52ea9d3217d2dc9 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 17 Feb 2025 19:48:59 +0100 Subject: [PATCH 6/6] update upload-artifacts to v4 --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e0b23ba7..6cd3250e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -59,7 +59,7 @@ jobs: pytest -v --cov --color=yes --cov-report=xml - name: Archive figures generated during testing if: always() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: visual_test_results_${{ matrix.os }}-python${{ matrix.python }} path: /home/runner/work/spatialdata-plot/spatialdata-plot/tests/figures/*