Skip to content

coloring shapes by categorical variable - #153

Merged
timtreis merged 13 commits into
mainfrom
bugfix/145_palette_in_render_shapes
Oct 4, 2023
Merged

coloring shapes by categorical variable#153
timtreis merged 13 commits into
mainfrom
bugfix/145_palette_in_render_shapes

Conversation

@Sonja-Stockhaus

Copy link
Copy Markdown
Collaborator

This should fix multiple problems when coloring shapes, namely:

# data reading, preprocessing
sdata = sd.read_zarr("visium_associated_xenium_io.zarr")
sdata.table.obs["cluster"] = "a"
sdata.table.obs.loc[sdata.table.obs.array_row==1.0, "cluster"] = "b"
sdata.table.obs.loc[sdata.table.obs.array_row==47.0, "cluster"] = "c"
sdata.table.obs.cluster = sdata.table.obs.cluster.astype("category")

1 Incorrect legend coloring when using the groups argument:

sdata.pl.render_shapes('CytAssist_FFPE_Human_Breast_Cancer', color="cluster", groups=["c", "b"]).pl.show("global")

grafik

2 palette parameter in render_shapes doesn't have influence on the plot (= #145)

sdata.pl.render_shapes('CytAssist_FFPE_Human_Breast_Cancer', color="cluster", groups=["c", "b"], palette=ListedColormap(['blue', 'red'])).pl.show("global")

grafik

Plus some downstream bugfixes that came up after that

@codecov-commenter

codecov-commenter commented Sep 1, 2023

Copy link
Copy Markdown

Codecov Report

Merging #153 (40288fe) into main (88f6928) will increase coverage by 4.10%.
The diff coverage is 78.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   74.88%   78.98%   +4.10%     
==========================================
  Files          11       11              
  Lines        1298     1342      +44     
==========================================
+ Hits          972     1060      +88     
+ Misses        326      282      -44     
Files Changed Coverage Δ
src/spatialdata_plot/pl/utils.py 68.87% <73.68%> (+7.72%) ⬆️
src/spatialdata_plot/pl/render.py 89.26% <79.41%> (+1.47%) ⬆️
src/spatialdata_plot/pl/basic.py 85.63% <100.00%> (ø)
src/spatialdata_plot/pl/render_params.py 100.00% <100.00%> (ø)

@timtreis

timtreis commented Sep 4, 2023

Copy link
Copy Markdown
Member

Is this bug already solved? Or were these diagnostic plots?

@Sonja-Stockhaus

Copy link
Copy Markdown
Collaborator Author

Bug is solved, I still want to add some tests

@Sonja-Stockhaus
Sonja-Stockhaus marked this pull request as ready for review September 4, 2023 08:51
@Sonja-Stockhaus
Sonja-Stockhaus marked this pull request as draft September 4, 2023 15:33
@Sonja-Stockhaus Sonja-Stockhaus linked an issue Sep 5, 2023 that may be closed by this pull request
@Sonja-Stockhaus
Sonja-Stockhaus marked this pull request as ready for review September 5, 2023 09:25
palette = (
ListedColormap(dict.fromkeys(color_vector)) if render_params.palette is None else render_params.palette
)
# remove the color of NaN values, else it might be assigned to a category

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to have this comment directly next to the dict.from_keys

color_map = dict(zip(categories, _get_colors_for_categorical_obs(categories)))
color_map = dict(zip(categories, _get_colors_for_categorical_obs(categories, palette)))
# color_map = _get_palette(
# adata=adata, cluster_key=value_to_plot, categories=categories, palette=palette, alpha=alpha

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment plz

Comment thread tests/pl/test_render_shapes.py Outdated
)

sdata_blobs.pl.render_shapes(
"blobs_polygons", color="cluster", groups=["c2", "c1"], palette=ListedColormap(["green", "yellow"])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take care of the ListedColormap conversion inside the code, it's unintuitive to have the user feed it in.

Comment thread CHANGELOG.md Outdated
- Multipolygons are now handled correctly (#93)
- Legend order is now deterministic (#143)
- Images no longer normalised by default (#150)
- Shapes can be colored by categorical variable with the `group` and `palette` arguments (#153)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make clear that groups is used to filter the data and palette is used to color the (potentially remaining) categories. Also, what happens if I add a palette argument to a continous variable?

@timtreis

timtreis commented Sep 5, 2023

Copy link
Copy Markdown
Member

Meeting notes @Sonja-Stockhaus

  • Implement also for labels using MibiTOF (this still depends on _filter_table_by_elements doesn't work for blobs dataset spatialdata#350)
  • Internalise ListedColormap conversion
  • one string is accepted for both arguments
  • Give tests better names -> Describe what they actually do, make distinction between groups and palette
  • What happens when user gives a palette arg for a continous variable? Throw error that tells user to use a cmap instead.
  • Docstrings
  • Change Changelog text

@Sonja-Stockhaus
Sonja-Stockhaus marked this pull request as draft September 5, 2023 16:39
Comment thread src/spatialdata_plot/pl/basic.py Outdated
@@ -183,8 +183,11 @@ def render_shapes(
Key in :attr:`anndata.AnnData.layers` or `None` for :attr:`anndata.AnnData.X`.
palette
Palette for discrete annotations, see :class:`matplotlib.colors.Colormap`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L185 and L186 disagree, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the :class:matplotlib.colors.Colormap?

Comment thread tests/pl/test_render_points.py Outdated
sdata_blobs.pl.render_points(elements="blobs_points").pl.show()

def test_plot_can_filter_with_groups(self, sdata_blobs: SpatialData):
sdata_blobs.pl.render_points(color="genes", groups="b", palette=["orange"]).pl.show()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the palette here to be just a string so we're also testing that minor functionality here

Comment thread src/spatialdata_plot/pl/utils.py Outdated
cmap = plt.get_cmap(palette)
palette = [to_hex(x) for x in cmap(color_idx, alpha=alpha)]
# cmap = plt.get_cmap(palette)
# palette = [to_hex(x) for x in cmap(color_idx, alpha=alpha)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comments

palette
Palette for discrete annotations, see :class:`matplotlib.colors.Colormap`.
Palette for discrete annotations. List of valid color names that should be used
for the categories (all or as specified by `groups`). For a single category,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good descriptions 👌

@timtreis
timtreis marked this pull request as ready for review October 4, 2023 14:18
@timtreis
timtreis merged commit 0e0ecb1 into main Oct 4, 2023
@timtreis
timtreis deleted the bugfix/145_palette_in_render_shapes branch October 4, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

palette argument has no effect on plotting colors in pl.render_shapes() groups still available for render_points()?

3 participants