rformassspectrometry / QFeatures

Quantitative features for mass spectrometry data
https://RforMassSpectrometry.github.io/QFeatures/
25 stars 7 forks source link

Issue with local filtering variables and values #208

Closed lgatto closed 7 months ago

lgatto commented 7 months ago

Positive control

Use case 1: this of course works

> data(feat1)
> feat1 <- aggregateFeatures(feat1, 1, fcol = "Sequence", name = "pep")
> feat1 <- aggregateFeatures(feat1, 2, fcol = "Protein", name = "prot")
> filterFeatures(feat1, ~  location == "Mitochondrion")
'location' found in 3 out of 3 assay(s)

An instance of class QFeatures containing 3 assays:
 [1] psms: SummarizedExperiment with 6 rows and 2 columns 
 [2] pep: SummarizedExperiment with 2 rows and 2 columns 
 [3] prot: SummarizedExperiment with 1 rows and 2 columns 

Use case 2: and also this

> target <- "Mitochondrion"
> filterFeatures(feat1, ~  location == target)
'location' found in 3 out of 3 assay(s)

An instance of class QFeatures containing 3 assays:
 [1] psms: SummarizedExperiment with 6 rows and 2 columns 
 [2] pep: SummarizedExperiment with 2 rows and 2 columns 
 [3] prot: SummarizedExperiment with 1 rows and 2 columns 

Issue

But!

> location <- 1
> filterFeatures(feat1, ~  location == target)
Error in .checkFilterVariables(rowData(object), vars) : No vars left.
> filterFeatures(feat1, ~  location == "Mitochondrion")
Error in .checkFilterVariables(rowData(object), vars) : No vars left.

(There's a unit test for this)

This issue was reported here.

Why is this?

In .checkFilterVariables(), there's a check that variables are present in the assay rowData. However, to account for use case 2 above, it is necessary to remove values that are defined in the calling environment (interactively, this is the global environment). But when doing so, we also remove variable names that exist in the calling (global) environment, which leads to deleting the filter variables.

Solution

We need to be able to distinguish filtering variable (names) from values, and only remove the latter.

lgatto commented 7 months ago

Fixed in commit #65938947627ebed331f2e5606ce4ee31b35282db