search

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

plot_pairwise_average_fst may change the order of the cohorts when using a cohort dict #540

Open jonbrenas opened 1 month ago

jonbrenas commented 1 month ago

Resolves #539. There might be other functions affected in the same way.

alimanfoo commented 1 month ago

Hi @jonbrenas, just to mention I've updated this branch to bring in some test fixes from master.

codecov[bot] commented 1 month ago

Codecov Report

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

Project coverage is 95.89%. Comparing base (dc89c9c) to head (3622b97). Report is 17 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #540 +/- ## ========================================== - Coverage 98.61% 95.89% -2.73% ========================================== Files 38 39 +1 Lines 3690 3821 +131 ========================================== + Hits 3639 3664 +25 - Misses 51 157 +106 ```

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

ahernank commented 1 month ago

@jonbrenas I've just re-ran the tests here to check they are all good now. It is only the coverage that needs a bit of additional work.

jonbrenas commented 1 month ago

Thanks @ahernank. I am not quite sure what the problem with codecov/project is, though. It looks like there is an indirect change that leaves a line uncovered in dipclust.py but I can't really figure out why that would be.

jonbrenas commented 4 weeks ago

Thanks, @leehart. The bottom-left half of the diagram wasn't being filled anymore. I also changed the list comprehension to .flatten(). (I personally find the list comprehension more readable but I see I am in the minority ;) )

leehart commented 4 weeks ago

Cool, thanks @jonbrenas !

I see there's suddenly a code coverage failure in CI, which I don't quite understand, which I think we've experienced before. I wonder if it's the product of something random. I can't seem to re-run it with any success. I think in the past I've just had to add more coverage to an unrelated part of the code to satisfy the threshold, but it's annoying.

jonbrenas commented 4 weeks ago

I think the problem is with dipcluster.py. It might be that the test doesn't cover some of the new functions.

leehart commented 4 weeks ago

Yes, I see that anoph/dipclust.py is the file with the biggest "Change %", i.e. -49.40%, with anoph/snp_frq.py having -4.79%.

I'm not sure what these figures mean!?

From https://docs.codecov.com/docs/coverage-percentages

Head coverage percent is the line coverage of all your files in your head commit. Patch coverage percent is the line coverage of all the lines changed in your commit. Change percent is the percent that the overall project line coverage has changed from base to head.

I guess if we can somehow claw back -2.72% test coverage, it should pass.