`pca`'s `cohorts` argument does not work the way it is expected to

[jonbrenas]

A cohorts parameter was added to pca but it is a bit confusing.

Most functions using a cohorts parameter, such as the diversity_stats which only works at the cohort level or snp_allele_frequencies which needs to split the data into cohorts, have a different use for cohorts than pca which uses it to downsample the data. Arguably, because the 2 parameters are semantically different, they should have different definitions and the current cohorts parameter works fine if it is set to a column name and only really fails if given a dictionary (or any other mapping, I would guess).

There are two options:

• change the cohorts parameter to be more restrictive
• change the code to cope with the more general definition of cohorts

[31puneet]

I think we can fix this by adding an isinstance(cohorts, Mapping) check before the groupby call in pca(), similar to how _locate_cohorts() already handles it

[jonbrenas]

Thanks @31puneet. I have not looked at the code yet. Wouldn't using _locate_cohorts() itself replace the need for the groupby?

[31puneet]

Yeah _locate_cohorts() should work nicely here. Just noticed it sorts cohort labels while the current groupby doesn't, which could affect cached results, but a version bump should handle that.

[fjcobu14]

Malaria research note: Analysis of 121 clinical trials for malaria in Kenya shows Phase 2 has the highest count at 33 studies.