From 29e70dd62ab27ed545a517b064f04e49ef6a9f02 Mon Sep 17 00:00:00 2001 From: David Collins Date: Thu, 25 Jul 2024 10:18:33 -0400 Subject: [PATCH 01/10] Add integration_checks.yaml --- .github/workflows/integration_checks.yaml | 45 +++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 .github/workflows/integration_checks.yaml diff --git a/.github/workflows/integration_checks.yaml b/.github/workflows/integration_checks.yaml new file mode 100644 index 00000000..81aeeb5e --- /dev/null +++ b/.github/workflows/integration_checks.yaml @@ -0,0 +1,45 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help +name: Integration Checks + +# because `main` is a protected branch this workflow is triggered when a PR +# is opened/updated and again when it is merged +on: + push: + branches: + - main + pull_request: + branches: + - main + +jobs: + check-package: + runs-on: ubuntu-latest + + steps: + # pull the latest changes from the repository down to the runner + - name: Checkout + uses: actions/checkout@v4 + + # install R-release and any system dependencies + - name: Setup R + uses: r-lib/actions/setup-r@v2 + with: + # specify additional repositories to pull dependencies not + # available on CRAN (i.e. `BPCells`) + extra-repositories: ${{ 'https://bnprks.r-universe.dev' }} + + # install R dependencies + - name: Install Dependencies + uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: + any::rcmdcheck + # installed packages are cached by default - force an upgrade to the + # latest version of all dependencies + upgrade: 'TRUE' + + # run CRAN checks - fails if any ERRORs or WARNINGs are raised in which + # case the `rcmdcheck` output will be uploaded as an artifact + - name: Run Checks + uses: r-lib/actions/check-r-package@v2 From e20135e13274aa3ed5892d4bf6d57a2735be1c8d Mon Sep 17 00:00:00 2001 From: David Collins Date: Thu, 25 Jul 2024 10:39:53 -0400 Subject: [PATCH 02/10] Update checks to suppress NOTEs ignored by CRAN --- .github/workflows/integration_checks.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/integration_checks.yaml b/.github/workflows/integration_checks.yaml index 81aeeb5e..9bae9b53 100644 --- a/.github/workflows/integration_checks.yaml +++ b/.github/workflows/integration_checks.yaml @@ -43,3 +43,10 @@ jobs: # case the `rcmdcheck` output will be uploaded as an artifact - name: Run Checks uses: r-lib/actions/check-r-package@v2 + env: + # suppress NOTEs that are accepted by CRAN + # see: https://www.rdocumentation.org/packages/rcmdcheck/versions/1.4.0/topics/rcmdcheck + _R_CHECK_PKG_SIZES_: false + _R_CHECK_RD_XREFS_: false + _R_CHECK_CRAN_INCOMING_NOTE_GNU_MAKE_: false + _R_CHECK_PACKAGE_DATASETS_SUPPRESS_NOTES_: true From b1feaf6205469338a92615009267bfef4dc1a545 Mon Sep 17 00:00:00 2001 From: David Collins Date: Thu, 25 Jul 2024 10:41:09 -0400 Subject: [PATCH 03/10] Add website build to integration checks --- .github/workflows/integration_checks.yaml | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/integration_checks.yaml b/.github/workflows/integration_checks.yaml index 9bae9b53..d5641796 100644 --- a/.github/workflows/integration_checks.yaml +++ b/.github/workflows/integration_checks.yaml @@ -35,6 +35,7 @@ jobs: with: extra-packages: any::rcmdcheck + any::pkgdown # installed packages are cached by default - force an upgrade to the # latest version of all dependencies upgrade: 'TRUE' @@ -44,9 +45,19 @@ jobs: - name: Run Checks uses: r-lib/actions/check-r-package@v2 env: - # suppress NOTEs that are accepted by CRAN - # see: https://www.rdocumentation.org/packages/rcmdcheck/versions/1.4.0/topics/rcmdcheck - _R_CHECK_PKG_SIZES_: false - _R_CHECK_RD_XREFS_: false - _R_CHECK_CRAN_INCOMING_NOTE_GNU_MAKE_: false - _R_CHECK_PACKAGE_DATASETS_SUPPRESS_NOTES_: true + # suppress NOTEs that are accepted by CRAN + # see: https://www.rdocumentation.org/packages/rcmdcheck/versions/1.4.0/topics/rcmdcheck + _R_CHECK_PKG_SIZES_: false + _R_CHECK_RD_XREFS_: false + _R_CHECK_CRAN_INCOMING_NOTE_GNU_MAKE_: false + _R_CHECK_PACKAGE_DATASETS_SUPPRESS_NOTES_: true + continue-on-error: true + + # build pkgdown site + - name: Build Website + run: | + pkgdown::build_site_github_pages( + new_process = FALSE, + install = FALSE + ) + shell: Rscript {0} From 4346ae96bd6c9a094bdf51d770346ed682e83525 Mon Sep 17 00:00:00 2001 From: David Collins Date: Tue, 30 Jul 2024 14:54:36 -0400 Subject: [PATCH 04/10] Add matrix strategy to check multiple R versions --- .github/workflows/integration_checks.yaml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration_checks.yaml b/.github/workflows/integration_checks.yaml index d5641796..d28c9d9f 100644 --- a/.github/workflows/integration_checks.yaml +++ b/.github/workflows/integration_checks.yaml @@ -14,17 +14,28 @@ on: jobs: check-package: + # system dependencies for cannot be automatically resolved by + # `r-lib/actions/setup-r@v2` for macos or windows - to avoid having to + # maintain separate logic to infer and install system of those operating + # systems we'll only run integration checks with the ubuntu runs-on: ubuntu-latest + # run integration checks with R-release, R-devel, and R-oldrelease + strategy: + matrix: + r-version: ['release', 'devel', 'oldrel'] + steps: # pull the latest changes from the repository down to the runner - name: Checkout uses: actions/checkout@v4 - # install R-release and any system dependencies + # install R and any system dependencies - name: Setup R uses: r-lib/actions/setup-r@v2 with: + # install the R version specified by the current strategy + r-version: ${{ matrix.r-version }} # specify additional repositories to pull dependencies not # available on CRAN (i.e. `BPCells`) extra-repositories: ${{ 'https://bnprks.r-universe.dev' }} From e856d008d36832445fde1e7e8b5ee463cdf662d3 Mon Sep 17 00:00:00 2001 From: David Collins Date: Thu, 1 Aug 2024 10:27:58 -0400 Subject: [PATCH 05/10] Bump version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 3868ce70..bd905595 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: SeuratObject Type: Package Title: Data Structures for Single Cell Data -Version: 5.0.2 +Version: 5.0.99.9000 Authors@R: c( person(given = 'Paul', family = 'Hoffman', email = 'hoff0792@alumni.umn.edu', role = 'aut', comment = c(ORCID = '0000-0002-7693-8957')), person(given = 'Rahul', family = 'Satija', email = 'seurat@nygenome.org', role = c('aut', 'cre'), comment = c(ORCID = '0000-0001-9448-8833')), From 4f52ddf21e6ab24888f01e507cd739c030f8ca6a Mon Sep 17 00:00:00 2001 From: David Collins Date: Wed, 26 Jun 2024 07:54:54 -0400 Subject: [PATCH 06/10] Fix rownames length error in subset.StdAssay --- R/assay5.R | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/R/assay5.R b/R/assay5.R index f5652219..9e41b5d5 100644 --- a/R/assay5.R +++ b/R/assay5.R @@ -2395,7 +2395,8 @@ subset.StdAssay <- function( if (is.numeric(x = features)) { features <- Features(x = x, layer = NA)[features] } - features <- intersect(x = features, y = Features(x = x, layer = NA)) + all_features <- Features(x = x, layer = NA) + features <- intersect(x = features, y = all_features) if (!length(x = features)) { stop("None of the features provided found in this assay", call. = FALSE) } @@ -2411,12 +2412,6 @@ subset.StdAssay <- function( for (lyr in setdiff(x = layers.all, y = layers)) { LayerData(object = x, layer = lyr) <- NULL } - # Subset feature-level metadata - mfeatures <- MatchCells( - new = Features(x = x, layer = NA), - orig = features, - ordered = TRUE - ) # Perform the subsets for (l in layers) { lcells <- MatchCells( @@ -2448,6 +2443,13 @@ subset.StdAssay <- function( for (i in c('cells', 'features')) { slot(object = x, name = i) <- droplevels(x = slot(object = x, name = i)) } + # Subset feature-level metadata + mfeatures <- MatchCells( + new = all_features, + # in case any features were found in a only one layer and it was dropped + orig = intersect(features, Features(x = x, layer = NA)), + ordered = TRUE + ) slot(object = x, name = 'meta.data') <- slot( object = x, name = 'meta.data' From d6777f418c8e30bfb677aa42912a259936f82a39 Mon Sep 17 00:00:00 2001 From: David Collins Date: Wed, 26 Jun 2024 08:10:42 -0400 Subject: [PATCH 07/10] Allow `layer` to be only param for subset.StdAssay --- R/assay5.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/assay5.R b/R/assay5.R index 9e41b5d5..9a1309ca 100644 --- a/R/assay5.R +++ b/R/assay5.R @@ -2360,7 +2360,7 @@ subset.StdAssay <- function( layers = NULL, ... ) { - if (is.null(x = cells) && is.null(x = features)) { + if (is.null(cells) && is.null(features) && is.null(layers)){ return(x) } # Check the cells vector From 456e122ed1c4739b19459613b8be223338adf1cf Mon Sep 17 00:00:00 2001 From: David Collins Date: Wed, 26 Jun 2024 09:57:06 -0400 Subject: [PATCH 08/10] Fix rownames length error, properly Clean up subset.StdAssay --- R/assay5.R | 161 ++++++++++++++++++++++++++++------------------------- 1 file changed, 84 insertions(+), 77 deletions(-) diff --git a/R/assay5.R b/R/assay5.R index 9a1309ca..dc569c35 100644 --- a/R/assay5.R +++ b/R/assay5.R @@ -2360,101 +2360,108 @@ subset.StdAssay <- function( layers = NULL, ... ) { - if (is.null(cells) && is.null(features) && is.null(layers)){ - return(x) - } - # Check the cells vector - if (all(is.na(x = cells))) { - cells <- Cells(x = x, layer = NA) - } else if (any(is.na(x = cells))) { - warning( - "NAs passed in cells vector, removing NAs", - call. = FALSE, - immediate. = TRUE - ) - cells <- cells[!is.na(x = cells)] - } - if (is.numeric(x = cells)) { - cells <- Cells(x = x, layer = NA)[cells] - } - cells <- intersect(x = Cells(x = x, layer = NA), y = cells) - if (!length(x = cells)) { - stop("None of the cells provided found in this assay", call. = FALSE) - } - # Check the features vector - if (all(is.na(x = features))) { - features <- Features(x = x, layer = NA) - } else if (any(is.na(x = features))) { - warning( - "NAs passed in features vector, removing NAs", - call. = FALSE, - immediate. = TRUE - ) - features <- features[!is.na(x = features)] + # define an inner function to validate the `cells` and `features` params + .validate_param <- function(name, values, allowed) { + # if `values` is null or contains only null values, keep all allowed values + if (all(is.na(values))) { + values <- allowed + } else if (any(is.na(x = values))) { + # if any values are NA, issue a warning and remove NAs + warning( + paste0("NAs passed in ", name, " vector, removing NAs"), + call. = FALSE, + immediate. = TRUE + ) + # and drop null values from `values` + values <- values[!is.na(x = values)] + } + # if `values` is numeric, treat them as indices + if (is.numeric(values)) { + values <- allowed[values] + } + # ensure `values` are in the allowed set + values <- intersect(values, allowed) + # if no valid values remain, stop execution with an error + if (!length(values)) { + stop(paste0("None of the ", name, " provided found in this assay"), call. = FALSE) + } + return(values) } - if (is.numeric(x = features)) { - features <- Features(x = x, layer = NA)[features] + + # if no subsetting is specified, return the original object + if (is.null(cells) && is.null(features) && is.null(layers)) { + return(x) } + + # validate and filter cells + all_cells <- Cells(x) + cells <- .validate_param("cells", cells, all_cells) + # validate and filter features all_features <- Features(x = x, layer = NA) - features <- intersect(x = features, y = all_features) - if (!length(x = features)) { - stop("None of the features provided found in this assay", call. = FALSE) - } - # Check the layers - layers.all <- Layers(object = x) - layers <- layers %||% layers.all + features <- .validate_param("features", features, all_features) + # validate and filter layers + all_layers <- Layers(object = x) + layers <- layers %||% all_layers layers <- match.arg( arg = layers, - choices = layers.all, + choices = all_layers, several.ok = TRUE ) - # Remove unused layers - for (lyr in setdiff(x = layers.all, y = layers)) { - LayerData(object = x, layer = lyr) <- NULL - } - # Perform the subsets - for (l in layers) { - lcells <- MatchCells( - new = Cells(x = x, layer = l), + + # subset cells and features layer by layer + for (layer_name in all_layers) { + # maybe drop the layer + if (!layer_name %in% layers) { + LayerData(x, layer = layer_name) <- NULL + next + } + # otherwise, filter the the layer's cells and features + # `MatchCells` is a bit of a misnomer - assuming that `new` is a + # subset of `old`, the function returns a list of indices mapping + # the values of `new` to their order in `orig` + layer_cells <- MatchCells( + new = Cells(x = x, layer = layer_name), orig = cells, ordered = TRUE ) - lfeatures <- MatchCells( - new = Features(x = x, layer = l), + layer_features <- MatchCells( + new = Features(x = x, layer = layer_name), orig = features, ordered = TRUE ) - if (is.null(x = lcells) || is.null(x = features)) { - LayerData(object = x, layer = l) <- NULL - } else { - LayerData(object = x, layer = l) <- LayerData( - object = x, - layer = l, - cells = lcells, - features = lfeatures - ) - } - } - slot(object = x, name = 'cells') <- droplevels(x = slot( - object = x, - name = 'cells' - )) - # Update the cell/feature maps - for (i in c('cells', 'features')) { - slot(object = x, name = i) <- droplevels(x = slot(object = x, name = i)) + # if no valid cells or features, drop the layer data + if (is.null(layer_cells) || is.null(layer_features)) { + LayerData(object = x, layer = layer_name) <- NULL + next + } + # otherwise, apply the subset + LayerData(object = x, layer = layer_name) <- LayerData( + object = x, + layer = layer_name, + cells = layer_cells, + features = layer_features + ) } - # Subset feature-level metadata + + # clean up the cells and features slots + slot(x, name = "cells") <- droplevels(slot(x, name = "cells")) + slot(x, name = "features") <- droplevels(slot(x, name = "features")) + + # in case any features were found in a only one layer and it was dropped + # in the previous loop, we need to make sure our feature list is updated + features <- intersect(features, Features(x = x, layer = NA)) + # update the features to match the valid list - see note above on `MatchCells` mfeatures <- MatchCells( new = all_features, - # in case any features were found in a only one layer and it was dropped - orig = intersect(features, Features(x = x, layer = NA)), + orig = features, ordered = TRUE ) - slot(object = x, name = 'meta.data') <- slot( - object = x, - name = 'meta.data' - )[mfeatures, , drop = FALSE] - validObject(object = x) + # subset the meta.data slot accordingly + slot(x, name = "meta.data") <- slot(x, name = "meta.data")[mfeatures, , drop = FALSE] + + # ensure the object is valid + validObject(x) + return(x) } From 45fd0a033899349f3696149ef6df7c79220409eb Mon Sep 17 00:00:00 2001 From: David Collins Date: Fri, 2 Aug 2024 16:19:21 -0400 Subject: [PATCH 09/10] Update NEWS --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index d33d5b18..751f7ad5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# Unreleased + +## Changes: +- Fix bug in `subset` - prevent `invalid 'row.names' length` error when one or more layers are dropped during feature-level subsetting (#214) + # SeuratObject 5.0.2 ## Changes: From 5fb74ac2a407248c5953d8b59834e60ecbdd29c9 Mon Sep 17 00:00:00 2001 From: David Collins Date: Fri, 2 Aug 2024 16:19:29 -0400 Subject: [PATCH 10/10] Bump version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index bd905595..7e8f2525 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: SeuratObject Type: Package Title: Data Structures for Single Cell Data -Version: 5.0.99.9000 +Version: 5.0.99.9001 Authors@R: c( person(given = 'Paul', family = 'Hoffman', email = 'hoff0792@alumni.umn.edu', role = 'aut', comment = c(ORCID = '0000-0002-7693-8957')), person(given = 'Rahul', family = 'Satija', email = 'seurat@nygenome.org', role = c('aut', 'cre'), comment = c(ORCID = '0000-0001-9448-8833')),