thelovelab / fishpond

Differential expression and allelic analysis, nonparametric statistics
https://thelovelab.github.io/fishpond
27 stars 9 forks source link

matrixStats::rowRanks(): Specify argument 'ties.method = "max"' to avoid being affected by future updates #5

Closed HenrikBengtsson closed 3 years ago

HenrikBengtsson commented 3 years ago

matrixStats will align the default ties.method for colRanks() and rowRanks() with that of base::rank(), cf. https://github.com/HenrikBengtsson/matrixStats/issues/142. Because of this, I ran a few reverse dependency checks to see what could break, and I spotted fishpond. It doesn't specify ties.method, so its results will be affected when we change the default:

https://github.com/mikelove/fishpond/search?q=rowRanks

mikelove commented 3 years ago

Thanks @HenrikBengtsson for the heads-up.

Because we never compute rowRanks on any vector that could have a tie, I think we can keep code as is (so going with the new default).

As in SAMseq, we add runif to anything we compute ties on.

HenrikBengtsson commented 3 years ago

Ok, good. However, I'll be deprecating, and then defunct:ing, the case when missing(ties.method) is true, for quite a while, before changing the default. This is to alert as many people as possible, so they can update any scripts using it. I wanna be conservative and minimize the risk for the change to go unnoticed "out there". So, you'll eventually have to specify the argument ... at least for quite a while.

mikelove commented 3 years ago

Oh I see, I'll switch to ties.method = "average" then in devel branch.