From 6ec3be06896bc3e4f7909bd7a6f6f4ec54762d72 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:15:13 +0000 Subject: [PATCH 1/2] Reimplement plotTIS()/plotRIC() on top of GCIMSSpectrumSet/GCIMSChromatogramSet plotTIS() and plotRIC() had near-identical, duplicated code to melt a matrix into long format and build a ggplot with SampleID coloring. Now both build a GCIMSSpectrumSet/GCIMSChromatogramSet from the (possibly sample-subsetted) matrix and delegate the actual plotting to plot.GCIMSSpectrumSet/plot.GCIMSChromatogramSet, removing the duplication. This also lets both gain a color_by parameter (defaulting to "SampleID", preserving prior behavior) to color by any pData() column, matching the annotation-aware plotting GCIMSChromatogramSet/ GCIMSSpectrumSet were built for. getTIS()/getRIC() themselves are untouched: they keep returning interpolated matrices via the joint per-sample extraction (extract_RIC_and_TIS), since that's still the right representation for a matrix API and is relied on directly by align-GCIMSDataset.R. Updated the existing plotTIS/plotRIC tests: plot.*Set() attaches data at the top-level ggplot object (ggplot(df) + geom_line(mapping = ...)), not per-layer like the old geom_line(data = ...) did, so tests now read p$data instead of p$layers[[1]]$data. Added a test for the new color_by parameter. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56 --- R/getTIS_getRIC-GCIMSDataset.R | 67 +++++++++---------- man/plotRIC-GCIMSDataset-method.Rd | 4 +- man/plotTIS-GCIMSDataset-method.Rd | 4 +- .../test-getTIS_getRIC-GCIMSDataset.R | 27 +++++--- 4 files changed, 57 insertions(+), 45 deletions(-) diff --git a/R/getTIS_getRIC-GCIMSDataset.R b/R/getTIS_getRIC-GCIMSDataset.R index 131f9469..1cfa79b6 100644 --- a/R/getTIS_getRIC-GCIMSDataset.R +++ b/R/getTIS_getRIC-GCIMSDataset.R @@ -35,12 +35,13 @@ setMethod("getRIC", "GCIMSDataset", function(object) { #' @param object A [GCIMSDataset] object #' @inheritParams dt_rt_range_normalization #' @param sample A number or a string with the sample index or name. If `NULL`, all samples are returned +#' @param color_by The name of a `pData(object)` column (or `"SampleID"`) used to color the spectra #' @return The plot of the TIS #' @export setMethod( "plotTIS", "GCIMSDataset", - function(object, dt_range = NULL, sample = NULL) { + function(object, dt_range = NULL, sample = NULL, color_by = "SampleID") { tis <- getTIS(object) dt <- dtime(object) sample_names <- sampleNames(object) @@ -49,22 +50,20 @@ setMethod( } sample_idx <- sample_name_or_number_to_both(sample, sample_names) idx <- dt_rt_range_normalization(dt = dt, dt_range = dt_range) - tis_long <- reshape2::melt(tis[sample_idx$idx, idx$dt_logical, drop = FALSE], value.name = "TIS") - gplt <- ggplot2::ggplot() + - ggplot2::geom_line( - data = tis_long, - mapping = ggplot2::aes( - x = .data$drift_time_ms, - y = .data$TIS, - color = .data$SampleID - ) - ) + - ggplot2::labs( - x = "Drift time (ms)", - y = "TIS Intensity (a.u.)", - color = "SampleID" - ) - gplt + tis_sub <- tis[sample_idx$idx, idx$dt_logical, drop = FALSE] + dt_sub <- dt[idx$dt_logical] + + pd <- pData(object) + spectra <- stats::setNames( + purrr::map(rownames(tis_sub), function(sample_id) { + GCIMSSpectrum(drift_time = dt_sub, intensity = unname(tis_sub[sample_id, ]), description = sample_id) + }), + rownames(tis_sub) + ) + spectrum_set <- GCIMSSpectrumSet(spectra = spectra, pData = pd[pd$SampleID %in% rownames(tis_sub), , drop = FALSE]) + + plot(spectrum_set, color_by = color_by) + + ggplot2::labs(y = "TIS Intensity (a.u.)") } ) @@ -73,12 +72,13 @@ setMethod( #' @param object A [GCIMSDataset] object #' @inheritParams dt_rt_range_normalization #' @param sample A number or a string with the sample index or name. If `NULL`, all samples are returned +#' @param color_by The name of a `pData(object)` column (or `"SampleID"`) used to color the chromatograms #' @return A plot #' @export setMethod( "plotRIC", "GCIMSDataset", - function(object, rt_range = NULL, sample = NULL) { + function(object, rt_range = NULL, sample = NULL, color_by = "SampleID") { ric <- getRIC(object) rt <- rtime(object) sample_names <- sampleNames(object) @@ -87,23 +87,20 @@ setMethod( } sample_idx <- sample_name_or_number_to_both(sample, sample_names) idx <- dt_rt_range_normalization(rt = rt, rt_range = rt_range) - ric_long <- reshape2::melt(ric[sample_idx$idx, idx$rt_logical, drop = FALSE], value.name = "RIC") - - gplt <- ggplot2::ggplot() + - ggplot2::geom_line( - data = ric_long, - mapping = ggplot2::aes( - x = .data$retention_time_s, - y = .data$RIC, - color = .data$SampleID - ) - ) + - ggplot2::labs( - x = "Retention time (s)", - y = "RIC Intensity (a.u.)", - color = "SampleID" - ) - gplt + ric_sub <- ric[sample_idx$idx, idx$rt_logical, drop = FALSE] + rt_sub <- rt[idx$rt_logical] + + pd <- pData(object) + chromatograms <- stats::setNames( + purrr::map(rownames(ric_sub), function(sample_id) { + GCIMSChromatogram(retention_time = rt_sub, intensity = unname(ric_sub[sample_id, ]), description = sample_id) + }), + rownames(ric_sub) + ) + chromatogram_set <- GCIMSChromatogramSet(chromatograms = chromatograms, pData = pd[pd$SampleID %in% rownames(ric_sub), , drop = FALSE]) + + plot(chromatogram_set, color_by = color_by) + + ggplot2::labs(y = "RIC Intensity (a.u.)") } ) diff --git a/man/plotRIC-GCIMSDataset-method.Rd b/man/plotRIC-GCIMSDataset-method.Rd index 469dc8e2..71cbe5ab 100644 --- a/man/plotRIC-GCIMSDataset-method.Rd +++ b/man/plotRIC-GCIMSDataset-method.Rd @@ -4,7 +4,7 @@ \alias{plotRIC,GCIMSDataset-method} \title{Plot Reverse Ion Chromatograms} \usage{ -\S4method{plotRIC}{GCIMSDataset}(object, rt_range = NULL, sample = NULL) +\S4method{plotRIC}{GCIMSDataset}(object, rt_range = NULL, sample = NULL, color_by = "SampleID") } \arguments{ \item{object}{A \link{GCIMSDataset} object} @@ -12,6 +12,8 @@ \item{rt_range}{The minimum and maximum retention times to extract (length 2 vector)} \item{sample}{A number or a string with the sample index or name. If \code{NULL}, all samples are returned} + +\item{color_by}{The name of a \code{pData(object)} column (or \code{"SampleID"}) used to color the chromatograms} } \value{ A plot diff --git a/man/plotTIS-GCIMSDataset-method.Rd b/man/plotTIS-GCIMSDataset-method.Rd index 90e7d832..681d9f98 100644 --- a/man/plotTIS-GCIMSDataset-method.Rd +++ b/man/plotTIS-GCIMSDataset-method.Rd @@ -4,7 +4,7 @@ \alias{plotTIS,GCIMSDataset-method} \title{Plot Total Ion Spectra} \usage{ -\S4method{plotTIS}{GCIMSDataset}(object, dt_range = NULL, sample = NULL) +\S4method{plotTIS}{GCIMSDataset}(object, dt_range = NULL, sample = NULL, color_by = "SampleID") } \arguments{ \item{object}{A \link{GCIMSDataset} object} @@ -12,6 +12,8 @@ \item{dt_range}{The minimum and maximum drift times to extract (length 2 vector)} \item{sample}{A number or a string with the sample index or name. If \code{NULL}, all samples are returned} + +\item{color_by}{The name of a \code{pData(object)} column (or \code{"SampleID"}) used to color the spectra} } \value{ The plot of the TIS diff --git a/tests/testthat/test-getTIS_getRIC-GCIMSDataset.R b/tests/testthat/test-getTIS_getRIC-GCIMSDataset.R index 3590dfdc..cf40c084 100644 --- a/tests/testthat/test-getTIS_getRIC-GCIMSDataset.R +++ b/tests/testthat/test-getTIS_getRIC-GCIMSDataset.R @@ -10,7 +10,8 @@ make_dataset <- function() { } s1 <- make_s(2, 25) s2 <- make_s(2.2, 25) - GCIMSDataset$new_from_list(samples = list(s1 = s1, s2 = s2), on_ram = TRUE, scratch_dir = NULL) + pd <- data.frame(SampleID = c("s1", "s2"), Group = c("A", "B")) + GCIMSDataset$new_from_list(samples = list(s1 = s1, s2 = s2), pData = pd, on_ram = TRUE, scratch_dir = NULL) } test_that("getTIS()/getRIC() extract per-sample TIS and RIC matrices, keyed by SampleID", { @@ -39,8 +40,8 @@ test_that("plotTIS() returns a ggplot with one line per sample over the full dri expect_s3_class(p, "ggplot") expect_equal(p$labels$x, "Drift time (ms)") expect_equal(p$labels$y, "TIS Intensity (a.u.)") - expect_setequal(as.character(unique(p$layers[[1]]$data$SampleID)), c("s1", "s2")) - expect_equal(range(p$layers[[1]]$data$drift_time_ms), c(0, 4)) + expect_setequal(as.character(unique(p$data$SampleID)), c("s1", "s2")) + expect_equal(range(p$data$drift_time_ms), c(0, 4)) }) test_that("plotTIS() restricts to dt_range and a single sample when given", { @@ -48,8 +49,8 @@ test_that("plotTIS() restricts to dt_range and a single sample when given", { p <- plotTIS(ds, dt_range = c(1, 3), sample = "s1") - expect_equal(as.character(unique(p$layers[[1]]$data$SampleID)), "s1") - expect_equal(range(p$layers[[1]]$data$drift_time_ms), c(1, 3)) + expect_equal(as.character(unique(p$data$SampleID)), "s1") + expect_equal(range(p$data$drift_time_ms), c(1, 3)) }) test_that("plotRIC() returns a ggplot with one line per sample over the full retention time range by default", { @@ -60,7 +61,7 @@ test_that("plotRIC() returns a ggplot with one line per sample over the full ret expect_s3_class(p, "ggplot") expect_equal(p$labels$x, "Retention time (s)") expect_equal(p$labels$y, "RIC Intensity (a.u.)") - expect_setequal(as.character(unique(p$layers[[1]]$data$SampleID)), c("s1", "s2")) + expect_setequal(as.character(unique(p$data$SampleID)), c("s1", "s2")) }) test_that("plotRIC() restricts to rt_range and a single sample when given", { @@ -68,6 +69,16 @@ test_that("plotRIC() restricts to rt_range and a single sample when given", { p <- plotRIC(ds, rt_range = c(10, 30), sample = 1) - expect_equal(as.character(unique(p$layers[[1]]$data$SampleID)), "s1") - expect_equal(range(p$layers[[1]]$data$retention_time_s), c(10, 30)) + expect_equal(as.character(unique(p$data$SampleID)), "s1") + expect_equal(range(p$data$retention_time_s), c(10, 30)) +}) + +test_that("plotTIS()/plotRIC() can color by a pData column instead of SampleID", { + ds <- make_dataset() + + p_tis <- plotTIS(ds, color_by = "Group") + p_ric <- plotRIC(ds, color_by = "Group") + + expect_equal(sort(unique(p_tis$data$Group)), c("A", "B")) + expect_equal(sort(unique(p_ric$data$Group)), c("A", "B")) }) From a2c70471551bbd4a86150f5c2b0351a4d2c866d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 17:18:06 +0000 Subject: [PATCH 2/2] Add plotTIS()/plotRIC() for GCIMSSample; share RIP-position logic plotTIS/plotRIC previously only had GCIMSDataset methods, unlike every other TIS/RIC-adjacent generic (getChromatogram, getSpectrum, smooth, baseline, findPeaks), which work on both GCIMSSample and GCIMSDataset. Add the GCIMSSample methods, mirroring the dataset-level ones: wrap the existing getTIS()/getRIC() output in a GCIMSSpectrum/GCIMSChromatogram (populated the same way getSpectrum()/getChromatogram() do) and delegate to their existing plot() methods. RIC is extracted from a single drift-time row (the RIP position), not aggregated across a range, so its chromatogram's drift_time_idx/ drift_time_ms are set to that one accurate value (via dt_rt_range_normalization()'s existing single-index support) rather than a misleading range or NA. That RIP-position computation (which.max(rowSums(intensity(object)))) was previously duplicated in both getRIC() and alignDt() (the latter reaching it via getTIS() instead). Factored it into one unexported rip_position() helper used by both, plus the new plotRIC(), so there's a single definition instead of three. Simplified the vignette's two hand-rolled single-sample TIS/RIC plots (manual data.frame + ggplot, including a vestigial no-op dplyr::filter) into plotTIS()/plotRIC() calls now that they exist. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56 --- R/align-GCIMSSample.R | 3 +- R/getTIS_getRIC-GCIMSSample.R | 55 ++++++++++++++++++- man/plotRIC-GCIMSSample-method.Rd | 19 +++++++ man/plotTIS-GCIMSSample-method.Rd | 19 +++++++ .../testthat/test-getTIS_getRIC-GCIMSSample.R | 55 +++++++++++++++++++ vignettes/introduction-to-gcims.Rmd | 15 +---- 6 files changed, 149 insertions(+), 17 deletions(-) create mode 100644 man/plotRIC-GCIMSSample-method.Rd create mode 100644 man/plotTIS-GCIMSSample-method.Rd create mode 100644 tests/testthat/test-getTIS_getRIC-GCIMSSample.R diff --git a/R/align-GCIMSSample.R b/R/align-GCIMSSample.R index 5c3b2495..3986e7cc 100644 --- a/R/align-GCIMSSample.R +++ b/R/align-GCIMSSample.R @@ -102,9 +102,8 @@ methods::setMethod( #' @return The modified [GCIMSSample] #' @export alignDt <- function(object, rip_ref_ms) { - tis <- getTIS(object) dt <- dtime(object) - rip_pos_ms <- dt[which.max(tis)] + rip_pos_ms <- dt[rip_position(object)] Kcorr <- rip_ref_ms/rip_pos_ms int_mat <- intensity(object) diff --git a/R/getTIS_getRIC-GCIMSSample.R b/R/getTIS_getRIC-GCIMSSample.R index 616d4262..eb469a47 100644 --- a/R/getTIS_getRIC-GCIMSSample.R +++ b/R/getTIS_getRIC-GCIMSSample.R @@ -28,12 +28,63 @@ setMethod("getTIS", "GCIMSSample", function(object) { #' ric <- getRIC(s) setMethod("getRIC", "GCIMSSample", function(object) { intmat <- intensity(object) - tis <- rowSums(intmat) - ric_pos <- which.max(tis) + ric_pos <- rip_position(object) ric <- intmat[ric_pos, ] ric <- max(ric) - ric ric <- ric/sum(ric) ric }) +#' Drift time index of the Reactant Ion Peak (RIP) +#' +#' The RIP is the drift time with the highest total ion signal, summed +#' across all retention times (the `which.max()` of [getTIS()]). +#' +#' @param object A [GCIMSSample] object +#' @return An integer, the drift time index of the RIP +#' @noRd +rip_position <- function(object) { + which.max(rowSums(intensity(object))) +} + +#' Plot the total ion spectrum of a sample +#' @param object A [GCIMSSample] object +#' @inheritParams dt_rt_range_normalization +#' @return A ggplot2 plot object +#' @export +setMethod("plotTIS", "GCIMSSample", function(object, dt_range = NULL) { + dt <- dtime(object) + rt <- rtime(object) + idx <- dt_rt_range_normalization(dt, rt, dt_range = dt_range) + tis <- getTIS(object) + spec <- GCIMSSpectrum( + drift_time = dt[idx[["dt_logical"]]], + intensity = tis[idx[["dt_logical"]]], + retention_time_idx = unique(c(idx[["rt_idx_min"]], idx[["rt_idx_max"]])), + retention_time_s = unique(c(idx[["rt_s_min"]], idx[["rt_s_max"]])), + description = object@description + ) + plot(spec) + ggplot2::labs(y = "TIS Intensity (a.u.)") +}) + +#' Plot the reverse ion chromatogram of a sample +#' @param object A [GCIMSSample] object +#' @inheritParams dt_rt_range_normalization +#' @return A ggplot2 plot object +#' @export +setMethod("plotRIC", "GCIMSSample", function(object, rt_range = NULL) { + dt <- dtime(object) + rt <- rtime(object) + ric <- getRIC(object) + ric_pos <- rip_position(object) + idx <- dt_rt_range_normalization(dt, rt, dt_idx = ric_pos, rt_range = rt_range) + chrom <- GCIMSChromatogram( + retention_time = rt[idx[["rt_logical"]]], + intensity = ric[idx[["rt_logical"]]], + drift_time_idx = unique(c(idx[["dt_idx_min"]], idx[["dt_idx_max"]])), + drift_time_ms = unique(c(idx[["dt_ms_min"]], idx[["dt_ms_max"]])), + description = object@description + ) + plot(chrom) + ggplot2::labs(y = "RIC Intensity (a.u.)") +}) diff --git a/man/plotRIC-GCIMSSample-method.Rd b/man/plotRIC-GCIMSSample-method.Rd new file mode 100644 index 00000000..74d4c498 --- /dev/null +++ b/man/plotRIC-GCIMSSample-method.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/getTIS_getRIC-GCIMSSample.R +\name{plotRIC,GCIMSSample-method} +\alias{plotRIC,GCIMSSample-method} +\title{Plot the reverse ion chromatogram of a sample} +\usage{ +\S4method{plotRIC}{GCIMSSample}(object, rt_range = NULL) +} +\arguments{ +\item{object}{A \link{GCIMSSample} object} + +\item{rt_range}{The minimum and maximum retention times to extract (length 2 vector)} +} +\value{ +A ggplot2 plot object +} +\description{ +Plot the reverse ion chromatogram of a sample +} diff --git a/man/plotTIS-GCIMSSample-method.Rd b/man/plotTIS-GCIMSSample-method.Rd new file mode 100644 index 00000000..f5c0a2fe --- /dev/null +++ b/man/plotTIS-GCIMSSample-method.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/getTIS_getRIC-GCIMSSample.R +\name{plotTIS,GCIMSSample-method} +\alias{plotTIS,GCIMSSample-method} +\title{Plot the total ion spectrum of a sample} +\usage{ +\S4method{plotTIS}{GCIMSSample}(object, dt_range = NULL) +} +\arguments{ +\item{object}{A \link{GCIMSSample} object} + +\item{dt_range}{The minimum and maximum drift times to extract (length 2 vector)} +} +\value{ +A ggplot2 plot object +} +\description{ +Plot the total ion spectrum of a sample +} diff --git a/tests/testthat/test-getTIS_getRIC-GCIMSSample.R b/tests/testthat/test-getTIS_getRIC-GCIMSSample.R new file mode 100644 index 00000000..c3110364 --- /dev/null +++ b/tests/testthat/test-getTIS_getRIC-GCIMSSample.R @@ -0,0 +1,55 @@ +make_rip_sample <- function() { + dt <- seq(1, 10, by = 0.5) # 19 pts, dt[10] == 5.5 + rt <- seq(1, 60, by = 1) + data <- matrix(1, nrow = length(dt), ncol = length(rt)) + data[10, ] <- 1000 # unambiguous RIP row at dt = 5.5 + GCIMSSample(drift_time = dt, retention_time = rt, data = data, description = "s1") +} + +test_that("plotTIS(GCIMSSample) plots the full spectrum by default, with an accurate subtitle", { + s <- make_rip_sample() + + p <- plotTIS(s) + + expect_s3_class(p, "ggplot") + expect_equal(p$labels$x, "Drift time (ms)") + expect_equal(p$labels$y, "TIS Intensity (a.u.)") + expect_equal(p$labels$title, "s1") + expect_match(p$labels$subtitle, "Retention time 1 - 60 s") + expect_equal(unname(p$data$y), unname(getTIS(s))) +}) + +test_that("plotTIS(GCIMSSample) restricts the displayed drift time range without changing values", { + s <- make_rip_sample() + + p_full <- plotTIS(s) + p_sub <- plotTIS(s, dt_range = c(2, 8)) + + expect_equal(range(p_sub$data$x), c(2, 8)) + # Values within the window are identical to the unrestricted plot's values: + common <- merge(p_full$data, p_sub$data, by = "x") + expect_equal(common$y.x, common$y.y) +}) + +test_that("plotRIC(GCIMSSample) has an accurate single-value drift time subtitle (the RIP position)", { + s <- make_rip_sample() + + p <- plotRIC(s) + + expect_s3_class(p, "ggplot") + expect_equal(p$labels$x, "Retention time (s)") + expect_equal(p$labels$y, "RIC Intensity (a.u.)") + expect_equal(p$labels$title, "s1") + expect_equal(p$labels$subtitle, "Drift time 5.5 ms") +}) + +test_that("plotRIC(GCIMSSample) restricts the displayed retention time range without changing values", { + s <- make_rip_sample() + + p_full <- plotRIC(s) + p_sub <- plotRIC(s, rt_range = c(10, 30)) + + expect_equal(range(p_sub$data$x), c(10, 30)) + common <- merge(p_full$data, p_sub$data, by = "x") + expect_equal(common$y.x, common$y.y) +}) diff --git a/vignettes/introduction-to-gcims.Rmd b/vignettes/introduction-to-gcims.Rmd index dca72331..a2057761 100644 --- a/vignettes/introduction-to-gcims.Rmd +++ b/vignettes/introduction-to-gcims.Rmd @@ -121,24 +121,13 @@ plot(ket1, rt_range = c(0, 1000), dt_range = c(7.5, 17)) ``` ```{r} -dt_k1 <- dtime(ket1) -tis_k1 <- getTIS(ket1) - -ggplot(dplyr::filter(data.frame(x = dt_k1, y = tis_k1), x >1)) + - geom_line(aes(x = x, y = y)) + - scale_x_continuous(name = "Drift time (ms)", limits = c(7, 17)) + +plotTIS(ket1, dt_range = c(7, 17)) + scale_y_continuous(name = "Intensity (a.u)", trans = cubic_root_trans()) ``` ```{r} -rt_k1 <- rtime(ket1) -ric_k1 <- getRIC(ket1) - -ggplot(dplyr::filter(data.frame(x = rt_k1, y = ric_k1))) + - geom_line(aes(x = x, y = y)) + - scale_x_continuous(name = "Retention time (ms)", limits = c(55, 900)) + - scale_y_continuous(name = "Intensity (a.u)") +plotRIC(ket1, rt_range = c(55, 900)) ``` Plot the RIC and the TIS to get an overview of the dataset: