Closed lgatto closed 1 year ago
Yes I agree with you, I also often use x[, , nrows(x) > 0]
or x[, , ncols(x) > 0]
.
Implementing a function (eg dropEmptyAssays()
) is straightforward, but I'm wondering what is the benefit of keeping empty assays in the first place.
What do you think about modifying the subsetting method so that we automatically remove empty assays (maybe along with a warning or a message)? I remember this was previously the default behavior, but modifications in MultiAssayExperiment
changes that behavior.
Yes, we could change that behaviour, but in what context? [
, filterFeatures()
, ... others? all?
For example, looking at the code, I thought we could have [, , , drop = FALSE]
implement the current behaviour, and [, , , drop = TRUE]
what we are discussing here (currently, its drop = TRUE
which is ignored, I think).
What else could you suggest?
The changes above might take some time and come late for the new release (which it might be already), so a dropEmptyAssays()
might be an easy and quick solution.
Also, could you confirm that this won't have any impact on the assayLinks
. It should not, given that there aren't any features in these assays, but I prefer to double check.
I'm going to start work with something like this for now:
dropEmptyAssays <- function(object, dims = 1:2) {
stopifnot(inherits(object, "QFeatures"))
if (!all(dims %in% 1:2))
stop("Argument 'dims' must be in '1:2'.")
if (1 %in% dims)
object <- object[, , nrows(object) > 0]
if (2 %in% dims)
object <- object[, , ncols(object) > 0]
object
}
Ok thanks for the implementation! I think indeed this allows for a quick solution. Regarding your concerns about AssayLinks
, there is no problem since [ , , ]
should always return a QFeatures
object with valid AssayLinks
.
In my opinion (and honestly I need to look back in the code to check if that's not already the case), any subsetting or filtering (filterFeatures()
, filterNA()
, [, , ]
,...) should make use of a common underlying subset method. This would improve maintainability, provide consistent behavior (probably facilitating the use of drop = TRUE
), and ensure that any subsetting keeps valid AssayLinks
.
I'll send a PR with that function.
I think this can be closed.
I don't think we have a function that removes empty assays from a QFeatures object yet - something like this:
I think this would be handy.
@cvanderaa, what do you think?