fix table name and add unit test for colors passed via .uns

[ArneDefauw]

spatialdata_plot.pl.render._render_labels still had sdata.table , changed to sdata.tables[table_name], otherwise colors passed via .uns where ignored.

also in https://github.com/scverse/spatialdata-plot/blob/43f25f654e123ca3a6a5da0e499ef15f22d0ec27/src/spatialdata_plot/pl/utils.py#L854

I would remove the check cluster_key in adata.uns. Because of this I had to set

    `sdata_blobs[ "other_table" ].uns[ "category" ] = "__value__"`

as a placeholder in https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L219 .

I also stumbled upon a subtle bug while testing this feature. With the small fix in this PR, one can pass colors via .uns[ f"{cluster_key}_colors" ], and all works as expected, see https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L219

We get:

But if we do a bounding box query, we get:

While we expect:

In short, this is caused by https://github.com/scverse/spatialdata/blob/03d3be80fad69ff54097e90a9e80ad02e9e0e242/src/spatialdata/_utils.py#L203

also see https://github.com/scverse/anndata/issues/997

For details see https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L227 and possible workaround https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L252

Note that being able to plot a subset in the same colors as the orignal is a rather common use case. However, I am not sure where to fix. Maybe documenting the workaround suffices for now.

[LucaMarconato]

Hi @ArneDefauw, thanks for the contribution. Two initial comments:

• could you please open one issue describing what the PR addresses? This makes it easier to review and keeps a cleaner history of what a PR introduces. At the moment it's not immediate and one has to go and check the code and discussion. Or in alternative, without having to open an issue, can you please add a summary at the beginning of the PR of the issues being addressed? Thanks a lot!
• I see some tests are not passing, one seems to be due to a difference between an old plot and the new one, while other seems to be due to raised exception. Do you have fix for them or comments on them?

Thanks!

[ArneDefauw]

Hi @LucaMarconato ,

I've linked an issue https://github.com/scverse/spatialdata-plot/issues/414.

For the failed unit tests, there was a KeyError, because I overlooked that table_name can be None. I changed it in the PR.

For me all unit tests fail locally on the main branch of spatialdata-plot, so it is difficult to test. Therfore, I did not include base images for the unit tests I've added, because they would probably fail anyway in the CI/CD testing environment.

The unit tests I've added are for documentation of the issue, some of them can probably be removed when merging to main.

[LucaMarconato]

Thanks for updating the PR! Regarding the failing tests, could you please give a try to the approach described here? https://github.com/scverse/spatialdata-plot/blob/main/docs/contributing.md

Locally generally the tests are reproducible on Ubuntu, but for other OS I use a Docker image, as described in the link above.

[LucaMarconato]

My configuration with PyCharm is described here https://github.com/scverse/spatialdata-plot/pull/397.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.23%. Comparing base (43f25f6) to head (be331f3). Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
+ Coverage   83.83%   84.23%   +0.40%     
==========================================
  Files           8        8              
  Lines        1732     1732              
==========================================
+ Hits         1452     1459       +7     
+ Misses        280      273       -7     

Files with missing lines	Coverage Δ
src/spatialdata_plot/pl/utils.py	78.69% <ø> (+0.69%)	:arrow_up:

[ArneDefauw]

I managed to fix the tests (using a centOS server).

Note that I had to move this line https://github.com/scverse/spatialdata-plot/blob/478ab27f9e8a6e06fc28c411e7721f1332b34b66/tests/pl/test_render_labels.py#L16 inside https://github.com/scverse/spatialdata-plot/blob/478ab27f9e8a6e06fc28c411e7721f1332b34b66/tests/pl/test_render_labels.py#L217, respectively https://github.com/scverse/spatialdata-plot/blob/478ab27f9e8a6e06fc28c411e7721f1332b34b66/tests/pl/test_render_labels.py#L237

to have the same results when running one test versus all the tests, because its internal state was moving forward due to multiple calls to it. Because of this I also had to update this figure https://github.com/scverse/spatialdata-plot/blob/main/tests/_images/Labels_can_annotate_labels_with_table_layer.png

[LucaMarconato]

Fantastic! Thanks a lot!

[timtreis]

Closed in favour of https://github.com/scverse/spatialdata-plot/pull/492

Thank you for the work @ArneDefauw! Only closing this one because I couldn't rebase your fork on the current main branch