HenrikBengtsson / matrixStats

R package: Methods that Apply to Rows and Columns of Matrices (and to Vectors)
https://cran.r-project.org/package=matrixStats
202 stars 33 forks source link

colRanks2()/rowRanks2(): As colRanks()rowRanks() where 'ties.method' has the same default as rank() #251

Open HenrikBengtsson opened 3 months ago

HenrikBengtsson commented 3 months ago

Updating the default for argument ties.method for rowRanks() might be too risky for backward compatible reasons, cf. https://github.com/HenrikBengtsson/matrixStats/issues/142.

It could be quicker to just introduce rowRanks2() that uses the same default.

yaccos commented 3 months ago

I think that would be a little confusing to the users as the only difference between the functions would be the default argument. Writing rowRanks(ties.method = "average") requires some more keystrokes, but is much clearer.

HenrikBengtsson commented 3 months ago

We can work with CRAN and Bioconductor packages to use explicit arguments, but I'm concerned about all existing scripts and dynamic reports that will silently give the wrong answer when changing the default. We have no way to notify those cases.

HenrikBengtsson commented 3 months ago

Either way, we could make argument ties.method a required argument for the existing colRanks() and rowRanks().

We could start out by giving a .Deprecation() warning when missing(ties.method) is TRUE. We probably have to limit the number of warnings produced to avoid flooding end users, but also to avoid a performance loss. We could start out softly by only reporting once per session. Then we can slowly crank up the noise level over releases. Eventually, we can get to a point where move to a .Defunct() error. We should probably also allow for temporarily adjusting this behavior via R options and matching environment variables.