Filter RPCAInput

[sabujcb]

No description provided.

[antagomir]

Seems good! Some checks still fail?

[antagomir]

@TuomasBorman can we accept this now?

[TuomasBorman]

Hi, thanks!

This is not only realted to jointRPCA, but can be more generic. Could the function be subsetBy* or filterBy* instead of filterRPCAInput (related to (related to #735)

Can you check that the style of code and documentation aligns with the rest of the package?

Use generic methods.

Also, not sure if MultiAssayExperiment method is needed. That complicates stuff unnecessarily. For instance, now these same thresholds are used for every omics layer.

I also think this is unnecessary complex. The operation is very simple and can be achieved with less code

library(mia)

data(GlobalPatterns)

tse <- GlobalPatterns

subsetByAbundance <- function(
        tse,
        assay.type = "counts",
        min.sample.count = 0,
        min.feature.count = 0,
        min.feature.frequency = 0,
        ...
){
    mia:::.check_assay_present(assay.type, tse)
    if( !mia:::.is_an_integer(min.sample.count) ){
        stop(.)
    }
    if( !mia:::.is_an_integer(min.feature.count) ){
        stop(.)
    }
    if( !mia:::.is_a_numeric(min.feature.frequency) ){
        stop(".")
    }
    #
    tse <- .filter_based_on_abundance(
        tse,
        assay.type = assay.type,
        min.sample.count = min.sample.count,
        min.feature.count = min.feature.count,
        min.feature.frequency = min.feature.frequency,
        ...
    )
    return(tse)
}

.filter_based_on_abundance <- function(
        tse,
        assay.type = "counts",
        min.sample.count = 0,
        min.feature.count = 0,
        min.feature.frequency = 0,
        na.rm = FALSE,
        ...
){
    if( !mia:::.is_a_bool(na.rm) ){
        stop(".")
    }
    #
    mat <- assay(tse, assay.type)
    col_sums <- mat |> colSums(na.rm = na.rm)
    row_sums <- mat |> rowSums(na.rm = na.rm)
    row_frequency <- rowSums(mat > 0, na.rm = na.rm) / ncol(tse)
    
    col_index <- col_sums >= min.sample.count
    row_index <- row_sums >= min.feature.count & row_frequency > min.feature.frequency
    
    tse <- tse[row_index, col_index]
    
    return(tse)
}

subsetByAbundance(tse)

[antagomir]

1. OK but be clear on the manpage which Python/Gemelli methods this corresponds to if the name is different.
2. OK if there is still something to do (good to have a look).
3. I am not sure if it is needed but at least this clearly is a multi-omic method, i.e. the type of case where MAE is usually promoted for. We could/should discuss whether there are general guidelines on when to use or not use MAE with multiomic datasets. I would hope that we can merge this ASAP and switching from MAE to something else might delay this further.
4. OK to me at least.

Let us try to finalize this before the weekend if possible.

[TuomasBorman]

Hi @sabujcb ! I made some changes, sorry about that, I heard that this PR was urgent.

I added some comments.

The functionality was already there and working, but I would try to keep the functions as simple as possible. If the same functionality can be achieved with less code, that is often better: less is more. There are some functions in mia that are a bit over-engineered, but for the sake of readability and maintainability, we should try to avoid adding more of those. Simpler code is usually easier to understand, test, and maintain.

About the naming: we could think about more generic name. Now it refers to RPCA, but I see this as a more generic utility function of filtering, thus the name could reflect that. But we can change it later.

Thanks for this PR!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.59%. Comparing base (78047df) to head (93463ee).
⚠️ Report is 5 commits behind head on devel.

Additional details and impacted files

@@            Coverage Diff             @@
##            devel     #837      +/-   ##
==========================================
+ Coverage   69.22%   70.59%   +1.36%     
==========================================
  Files          55       62       +7     
  Lines        7104     7513     +409     
==========================================
+ Hits         4918     5304     +386     
- Misses       2186     2209      +23     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.