Skip to content

Implementation of aggregateSamples - #263

Merged
lgatto merged 11 commits into
rformassspectrometry:masterfrom
leopoldguyot:aggregateSamples
Sep 3, 2026
Merged

lgatto merged 11 commits into
rformassspectrometry:masterfrom
leopoldguyot:aggregateSamples

Conversation

@leopoldguyot

Copy link
Copy Markdown
Collaborator

Implementation of aggregateSamples function. It allows to aggregates the columns of multiples sets of one QFeatures. Methods available for SE or QFeatures.

Related issues: #262 #188 #215

@leopoldguyot

Copy link
Copy Markdown
Collaborator Author

There is one major issue:
When aggregating several sets at the same time, conflicting coldata data would need to be added to the global colData.
For instance let say we want to aggregate several sets by patients and cell types. For each set the colData columns that are invariants are kept, this means for instance that a run column (this column should contain the same value for all the samples of a set) will be kept. After that when we try to add the new aggregated sets to the QFeatures, the addAssay function will add the information of the colData of the new SummarizedExperiment to the global colData. But if a row is already present in the global colData and that the new set's colData contain clashing data, an error will be thrown. In our example, for one pair of patient/cell type, the run column will contain different values between the different new set (therefore throwing an error).

I can think of different way to fix the problem:

  • Only allow the use of the function on a single set (i = character(1))
  • Extract the colData of each new Set and remove columns with clashing information before adding them to the QFeatures. (That would add a lot of extra data manipulation)
  • Don't add individual colData to the SummarizedExperiment but modify the global colData directly.

The last option means that we cannot simply use the SummarizedExperiment method in the QFeatures method, we need to have a new function that apply the aggregation on a SE but without returning a colData.

What do you think @lgatto ?

@leopoldguyot
leopoldguyot marked this pull request as ready for review July 29, 2026 13:15
@leopoldguyot
leopoldguyot requested a review from lgatto July 29, 2026 13:49
Comment thread NEWS.md Outdated
Comment thread R/QFeatures-aggregation.R Outdated
Comment thread R/QFeatures-aggregation.R Outdated
Comment thread R/QFeatures-aggregation.R Outdated
Comment thread R/QFeatures-aggregation.R Outdated
@leopoldguyot

Copy link
Copy Markdown
Collaborator Author

In the new 8.1.0 roxygen release there is now a "support" for setAs.

r-lib/roxygen2#1909 :

roxygen2 didn't recognise setAs() or setGroupGeneric(), so a roxygen block in front of either produced an Rd file with no \usage — which current R-devel reports as Rd files without \usage. setAs() also got no method alias. Both now behave like setMethod() and setGeneric():

\name{coerce,numeric,Account-method}
\alias{coerce,numeric,Account-method}
\usage{
\S4method{coerce}{numeric,Account}(from, to = "Account", strict = TRUE)
}

The problem is that this add this new entry in the QFeatures-class.Rd file:
\S4method{coerce}{MultiAssayExperiment,QFeatures}(from, to = "QFeatures", strict = TRUE)

Which introduces new parameters that were not used in the setAs definition (to and strict). This triggers warning during checks:

❯ checking Rd \usage sections ... WARNING
Undocumented arguments in Rd file 'QFeatures-class.Rd'
‘from’ ‘to’ ‘strict’

Functions with \usage entries need to have the appropriate \alias
entries, and all their arguments documented.
The \usage entries must correspond to syntactically valid R code.
See chapter ‘Writing R documentation files’ in the ‘Writing R
Extensions’ manual.

Should I add these parameters to the setAs method or only document them @lgatto ?

@leopoldguyot
leopoldguyot requested a review from lgatto September 3, 2026 15:21
@lgatto
lgatto merged commit 05a7fff into rformassspectrometry:master Sep 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants