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.

Sign up for GitHub

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

Jump to bottom

matrixStats 1.0.0: Breaking changes #91

Closed
HenrikBengtsson opened this issue Jun 2, 2023 · 5 comments
Closed

matrixStats 1.0.0: Breaking changes #91

HenrikBengtsson opened this issue Jun 2, 2023 · 5 comments

Comments

@HenrikBengtsson
Copy link

Hello, matrixStats 1.0.0 is now on CRAN (as 2023-06-02). It comes with the following significant changes:

  1. useNames = TRUE is the new default for all functions.

  2. colQuantiles() and rowQuantiles() only sets quantile percentage names when useNames = TRUE, to align with how argument names of stats::quantile() works in base R. Previously, useNames = FALSE also had such names attributes.
@HenrikBengtsson HenrikBengtsson mentioned this issue Jun 2, 2023
Closed
29 tasks
@PeteHaitch
Copy link
Owner

Thanks Henrik.
I'll await how we resolve this in Bioconductor/MatrixGenerics#31 before updating DelayedMatrixStats.

@kstreet13 kstreet13 mentioned this issue Jun 13, 2023
Open
@LTLA
Copy link
Contributor

LTLA commented Jun 30, 2023

Looks like you need to propagate useNames = useNames at

DelayedMatrixStats/R/utils.R

Line 316 in 1664201

return(candidate(S, ...))

Otherwise, user-provided overrides aren't respected when the dispatcher decides it can skip block processing.

@PeteHaitch
Copy link
Owner

@LTLA Won't the user-provided useNames be passed via ...?

@LTLA
Copy link
Contributor

LTLA commented Jul 6, 2023

No, because .smart_seed_dispatcher() has an explicit useNames argument, so it gets pulled out of the ....

PeteHaitch added a commit that referenced this issue Jul 6, 2023
@PeteHaitch
Pass useNames to candidate()
deef35f 
- See #91 (comment)
PeteHaitch added a commit that referenced this issue Jul 6, 2023
@PeteHaitch
Pass useNames to candidate()
3374a5b 
- See #91 (comment)
PeteHaitch added a commit that referenced this issue Jul 25, 2023
@PeteHaitch
Devel matrix stats 1.0.0 resync (#96)
723028e 
* WIP: Re-sync with matrixStats v1.0.0 API

* Update dependencies and `devtools::document()`

* WIP: Tweaks to tests

- Skipping tests that are currently failing for reasons outside of **DelayedMatrixStats**

* Update .Rbuildignore

* Use `names()<-` instead of `setNames()`

* Re-enable tests of `colRanges()` and `rowRanges()`

- Enabled by fix to Bioconductor/DelayedArray#106

* Correct `useNames` behaviour in rowCummins()

* Bump minimum version of sparseMatrixStats

- const-ae/sparseMatrixStats#27 fixe remaining unit tests

* Regenerate README

* Version bump

* Pass `useNames` to `candidate()`

- See #91 (comment)
@PeteHaitch
Copy link
Owner

PeteHaitch commented Jul 25, 2023 •
edited
Loading

Addressed as part of #96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@PeteHaitch @HenrikBengtsson @LTLA