Skip to content

feat: add support for Chromatograms to chromatogram - #837

Merged
sneumann merged 10 commits into
develfrom
chromatograms
Aug 17, 2026
Merged

feat: add support for Chromatograms to chromatogram#837
sneumann merged 10 commits into
develfrom
chromatograms

Conversation

@jorainer

Copy link
Copy Markdown
Collaborator
  • chromatogram,XcmsExperiment and chromatogram,MsExperiment now allow to return the results as a Chromatograms object.

- `chromatogram,XcmsExperiment` and `chromatogram,MsExperiment` now allow to
  return the results as a `Chromatograms` object.
@jorainer

Copy link
Copy Markdown
Collaborator Author

@philouail , can you please have a look. I will work also on the chromPeakChromatograms() function and after I have that make a full PR.

@philouail philouail left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks completely fine to me, just that one comment. thanks

#'
#' @param dataOrigin `character` with the **unique** file names/data origin.
#'
#' @param isolationWindowTargetMz optional `numeric` of length equal to the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add @PARAM msLevel precising that it will be rep() to the length of the object

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, thanks!

jorainer added 3 commits July 23, 2026 14:54
- `chromPeakChromatograms()` can also return the EICs as a `Chromatograms`
  object.
- `featureChromatograms()` can now also return the results as a `Chromatograms`
  object.
@jorainer
jorainer marked this pull request as ready for review July 27, 2026 11:35
@jorainer
jorainer requested a review from philouail July 27, 2026 11:35
@jorainer

Copy link
Copy Markdown
Collaborator Author

OK, @philouail , now it's ready for review.

- Add support for `Chromatograms` to the `XcmsExperimentHdf5` result object.
- Add new methods `featureChromPeaks()` and `featurePeakidx()` to access the
  mapping between features and chromatographic peaks.
@jorainer

Copy link
Copy Markdown
Collaborator Author

Note: Chromatograms support now also added to XcmsExperimentHdf5 result objects.

@jorainer
jorainer requested a review from sneumann August 4, 2026 05:58
@jorainer

jorainer commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@sneumann , this PR is ready for review - and merge. it adds support for the new Chromatograms infrastructure to xcms (in addition to the still supported MChromatograms from the MSnbase package).

- Fix for issue #839: `chromPeakData<-` for `XcmsExperiment` supports
  `data.frame` and `DataFrame` as input.
@jorainer

jorainer commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Includes now also the fix for #839

@sneumann sneumann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, looks good, and only needs the updated Chromatograms on BioC. Yours, Steffen

features = character(),
mzmin = min, mzmax = max,
rtmin = min, rtmax = max,
chunkSize = 2L, ...,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know you can mix ... into the middle of named parameters.

Comment thread DESCRIPTION
MetaboCoreUtils (>= 1.11.2),
data.table
data.table,
Chromatograms (>= 1.3.3)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BioC devel currently has Chromatograms 1.3.2, but the PR works if installed from GH.

@sneumann
sneumann merged commit ba6f4a1 into devel Aug 17, 2026
3 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.

3 participants