theislab / scib

Benchmarking analysis of data integration tools
MIT License
283 stars 62 forks source link

`cluster_optimal_resolution()` assumes argument order which can be incorrect for asymmetric metrics #366

Closed lazappi closed 1 year ago

lazappi commented 1 year ago

cluster_optimal_resolution() passes arguments to the metric function by position rather than name.

https://github.com/theislab/scib/blob/82444efd3e4378c487001e701a4a1209ef954aaa/scib/metrics/clustering.py#L102

This doesn't matter for symmetric metrics like ARI and NMI but in other cases it is important that the label and cluster are passed to the right arguments. I'm not sure it's better to pass the arguments by name (which gives less chance for error by means the metric function has to use the same names) or just be clearly about which argument has to come first (and make sure it's consistent across scIB functions). Either way the metric argument documents needs updating (it still mentions group1 and group2 instead of label_key and cluster_key).

mumichae commented 1 year ago

Good point. I went with the version without passing argument names, assuming the documentation is sufficient.