Add option to use cohorts as colour or symbol in plot_haplotype_clustering

malariagen / malariagen-data-python

Analyse MalariaGEN data from Python

https://malariagen.github.io/malariagen-data-python/latest/

MIT License

13 stars 23 forks source link

Add option to use cohorts as colour or symbol in plot_haplotype_clustering #441

Closed jonbrenas closed 9 months ago

jonbrenas commented 10 months ago

This seems to work on tests. Samples that do not belong to any cohort are shown on the plot (with a different colour or symbol, depending on the option selected) but do not appear in the legend. I don't know if that's the behaviour we want.

review-notebook-app[bot] commented 10 months ago

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov[bot] commented 10 months ago

Codecov Report

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

Comparison is base (5198e85) 97.51% compared to head (c7531bd) 97.51%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #441 +/- ## ======================================= Coverage 97.51% 97.51% ======================================= Files 26 26 Lines 2090 2090 ======================================= Hits 2038 2038 Misses 52 52 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

leehart commented 10 months ago

CI picks up a bug in notebooks/plot_haplotype_clustering.ipynb

https://github.com/malariagen/malariagen-data-python/actions/runs/6751077172/job/18354571648?pr=441

ag3.plot_haplotype_clustering(
region="2R:28,480,000-28,490,000",
sample_sets=["3.0"],
sample_query="taxon == 'arabiensis'",
analysis="gamb_colu_arab",
cohorts="admin1_year",
cohort_size=None,
width=1000,
height=400,
)

TypeError: scatter() got an unexpected keyword argument 'cohorts'

This seems to be because all of the **plot_kwargs are being passed to px.scatter(), which I gather doesn't take a cohorts param.

jonbrenas commented 10 months ago

Sorry, a little bit of a git hiccough. It is better now.

alimanfoo commented 9 months ago

Thanks @jonbrenas. Can you remove all outputs from the notebook - we normally do that to keep the notebook files light.

alimanfoo commented 9 months ago

There's some reworking of the plot_haplotypes_clustering() function coming in via #450, so I have manually merged the changes from this PR into #450. I'll close here.