Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make a scalar center argument for matrix functions defunct #254

Closed
HenrikBengtsson opened this issue Apr 6, 2024 · 10 comments
Closed

Make a scalar center argument for matrix functions defunct #254

HenrikBengtsson opened this issue Apr 6, 2024 · 10 comments
Milestone

Comments

@HenrikBengtsson
Copy link
Owner

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

  • Using a scalar value for argument center of colSds(),
    rowSds(), colVars(), rowVars(), colMads(), rowMads(),
    colWeightedMads(), and rowWeightedMads() is now deprecated.

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 HenrikBengtsson added this to the Next release milestone Apr 6, 2024
@HenrikBengtsson
Copy link
Owner Author

@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
Copy link
Owner Author

@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
Copy link
Contributor

const-ae commented Apr 9, 2024

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

@PeteHaitch
Copy link
Contributor

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

HenrikBengtsson added a commit that referenced this issue Apr 10, 2024
@HenrikBengtsson
Copy link
Owner Author

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
Copy link
Contributor

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

@HenrikBengtsson
Copy link
Owner Author

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

@HenrikBengtsson
Copy link
Owner Author

HenrikBengtsson commented Apr 10, 2024

... 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
Copy link
Contributor

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 added a commit to PeteHaitch/DelayedMatrixStats that referenced this issue Apr 11, 2024
@PeteHaitch
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants