Conversation
harryhaller001
commented
Feb 22, 2026
harryhaller001
commented
- Use matplotlib-sankey to add sankey plotting option to clustree function
- Add example to docs
- Python 3.11 as minimal supported version, move to uv
- Fix older pre-commit hooks
- Version updates to github actions
maltekuehl
left a comment
maltekuehl
left a comment
There was a problem hiding this comment.
Thanks for contributing this! Some comments raised, but the diff on the main file is quite large, could you perhaps summarize the changes and the motivation?
pyproject.toml
Outdated
| dependencies = [ | ||
| "anndata", | ||
| "matplotlib", | ||
| "matplotlib-sankey>=0.3.2", |
There was a problem hiding this comment.
How flexible is matplotlib-sankey with major versions of the standard scientific Python packages? Just to see whether it should be an optional or a required dependency
There was a problem hiding this comment.
I added matplotlib sankey as extra dependency
| [tool.flit.sdist] | ||
| exclude = [ "docs/*", "test/*" ] | ||
| [tool.flit] | ||
| sdist.exclude = [ "docs/*", "test/*" ] |
There was a problem hiding this comment.
Perspectively, would be great to eliminate flit dependency and just move to uv as in other repos. Unrelated to this PR, just if you have it set up and would like to copy & past this from some other place. But can merge without it, too.
There was a problem hiding this comment.
Good idea. I would start a separate PR to migrate to uv build backend
There was a problem hiding this comment.
python-igraph is deprecated, should now use igraph
There was a problem hiding this comment.
igraph is a dev dependency. I resolve the deprecation by adding "scanpy[leiden]".
| show_fraction: bool = False, | ||
| show_cluster_keys: bool = True, | ||
| graph_plot_kwargs: dict | None = None, | ||
| transition_plot: Literal["network", "sankey"] = "network", |
There was a problem hiding this comment.
Could we condense here to sankey_kwargs and point to its docs? Quite a number of keywords for optional behavior
There was a problem hiding this comment.
Sure. Thats a good idea. I added sankey parameters as kwargs
2 participants
