Skip to content

coloring shapes by categorical variable #153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Oct 4, 2023

Conversation

Sonja-Stockhaus
Copy link
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
Copy link

codecov-commenter commented Sep 1, 2023

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
Copy link
Member

timtreis commented Sep 4, 2023

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

@Sonja-Stockhaus
Copy link
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
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
Member

Choose a reason for hiding this comment

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

remove comment plz

)

sdata_blobs.pl.render_shapes(
"blobs_polygons", color="cluster", groups=["c2", "c1"], palette=ListedColormap(["green", "yellow"])
Copy link
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.

CHANGELOG.md Outdated
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning][].
- 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
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
Copy link
Member

timtreis commented Sep 5, 2023

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
@@ -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
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
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?

@@ -21,3 +21,12 @@
class TestPoints(PlotTester, metaclass=PlotTesterMeta):
def test_plot_can_render_points(self, sdata_blobs: SpatialData):
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
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

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
Member

Choose a reason for hiding this comment

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

remove comments

@@ -182,9 +182,13 @@ def render_shapes(
layer
Key in :attr:`anndata.AnnData.layers` or `None` for :attr:`anndata.AnnData.X`.
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
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