search

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

Make a scalar `center` argument for matrix functions defunct #254

Closed HenrikBengtsson closed 3 months ago

HenrikBengtsson commented 3 months ago

We have had the following since matrixStats 0.58.0 (2021-01-26):

We should move to make it defunct by default.

I've started revdep checks with R_MATRIXSTATS_CENTER_ONSCALAR=defunct to see if there are any negative side effects.

HenrikBengtsson commented 3 months ago

@PeteHaitch, passing a scalar to argument center will soon be defunct. I noticed that you explicitly allow for scalars in https://github.com/PeteHaitch/DelayedMatrixStats/blob/20c2e278d9ccf503a49dd489dc3597a21443c01d/R/rowVars.R#L16-L25.

HenrikBengtsson commented 3 months ago

@const-ae, FYI, passing a scalar to argument center will soon be defunct; currently deprecated. I noticed that sparseMatrixStats accepts scalar center values, e.g.

> sparseMatrixStats::colVars(sparse_mat, center = 4)
[1] 17.7777778  0.0000000  0.4444444  1.8888889  1.0000000  0.0000000
const-ae commented 3 months ago

Thanks for the heads up. I made scalar 'center' arguments defunct in the devel version of sparseMatrixStats. This was, anyways, not supported.

PeteHaitch commented 3 months ago

Thanks for the heads up. I'll take care of it before the upcoming BioC release.

HenrikBengtsson commented 3 months ago

Thank you. I've checked that there are no negative revdep side effects, so this will now be the default in the next release.

PeteHaitch commented 3 months ago

Is the new release imminent? More exactly, is it likely to be before or after the upcoming BioC release?

HenrikBengtsson commented 3 months ago

I'm thinking before ... Would it matter, given that I don't find any revdep packages being affected?

HenrikBengtsson commented 3 months ago

... and in worst case scenario (for current Bioc release and likes), it can for now be reverted using:

R_MATRIXSTATS_CENTER_ONSCALAR=deprecated

or in R, by:

options(matrixStats.center.onScalar = "deprecated")

This will be documented in ?matrixStats::matrixStats.options.

PeteHaitch commented 3 months ago

Thanks for the extra info. I don't expect it to matter but it's helpful to me in prioritising tasks as the BioC release approaches.

PeteHaitch commented 3 months ago

Made the change for current devel (BioC 3.19) but not touched current release (BioC 3.18).