Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update plotSpectra environment to allow addFragments returned list #348

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update plotSpectra environment to allow addFragments returned list #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0ec9de
Update plotSpectra environment to allow addFragments returned list
guideflandre Jan 22, 2025
945ab98
Update based on PR remarks
guideflandre Jan 23, 2025
7caf05c
Remove unnecessare files Rproj and Rhistory
guideflandre Jan 24, 2025
4814b87
Delete Spectra.Rproj
guideflandre Jan 24, 2025
6487255
Delete .Rhistory
guideflandre Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .Rbuildignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ favicon
logo.png
_pkgdown.yml
^\.git$
^.*\.Rproj$
^\.Rproj\.user$
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ docs
local_data
*.o
*.so
.Rproj.user
*.Rproj
.Rhistory
39 changes: 30 additions & 9 deletions R/plotting-functions.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,18 @@ plotSpectra <- function(x, xlab = "m/z", ylab = "intensity", type = "h",
main <- rep(main[1], nsp)
if (nsp > 1)
par(mfrow = n2mfrow(nsp, asp = asp))
if (length(labels)) {
if (is.function(labels)) {
labels <- labels(x)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to call a labels() function on the full Spectra object x? with the original code, the function would have been called on each spectrum individually (in the .plot_single_spectrum() function), so this would also avoid the need to support labels being a list.

In essence, the original plotSpectra() would call the labels() function on each individual spectrum, thus IMHO it would only require a labels() function that is able to extract whatever should be added/labeled from the spectra or peaks variables of that one spectrum. Would that not be possible in your case?

Copy link
Author

Choose a reason for hiding this comment

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

The essence of having labels return a list is twofold:

  1. addFragments used to throw an error when multiple Spectra were called when asking for a modification. This was because addFragments only accepted Spectra objects of length 1.
  2. In my personal use case of variable modifications, returning a list was the best option I had (including said attribute).

Thus this fixes a current Error, and it sets potential for variable modification.

}
if (length(labels) != length(x))
stop("Please provide a list of annotations of 'length == length(x)'.")
} else {labels <- NULL}

for (i in seq_len(nsp))
.plot_single_spectrum(x[i], xlab = xlab, ylab = ylab, type = type,
xlim = xlim, ylim = ylim, main = main[i],
col = col[[i]], labels = labels,
col = col[[i]], labels = labels[[i]],
Copy link
Member

Choose a reason for hiding this comment

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

if I understand it, labels can be either 1 or equal to the length of x - so, I guess above you would need to ensure that if labels is of length 1 it gets repeated length(x) times - otherwise you would get here an out-of-bound error.

Copy link
Author

@guideflandre guideflandre Jan 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This needs to be subject of discussion:

  1. labels could be of length 1 or equal to x, so if it is 1, repeat it length(x) times
  2. only accept labels as a function (= addFragments) that returns a list with unique attributes of length equal length(x), or accept labels as a list of length equal to length(x).

In the above, I used option 2) as I also used it for cases where variable modifications where used: which is also why there is an attribute to the elements of the returned list of addFragments. The attribute is an integer that corresponds to the spectrum that is called (if plotSpectra(sp[1:4], labels = addFragments is called, then addFragments returns a list of 4 elements. Each element can be matched to its corresponding spectrum thanks to the attribute)

labelCex = labelCex, labelSrt = labelSrt,
labelAdj = labelAdj, labelPos = labelPos,
labelOffset = labelOffset, labelCol = labelCol,
Expand Down Expand Up @@ -259,9 +267,16 @@ plotSpectraOverlay <- function(x, xlab = "m/z", ylab = "intensity",
if (frame.plot)
box(...)
title(main = main, xlab = xlab, ylab = ylab, ...)
if (length(labels)) {
if (is.function(labels)) {
labels <- labels(x)
}
if (length(labels) != length(x))
stop("Please provide a list of annotations of 'length == length(x)'.")
} else {labels <- NULL}
for (i in seq_len(nsp))
.plot_single_spectrum(x[i], add = TRUE, type = type, col = col[[i]],
labels = labels, labelCex = labelCex,
labels = labels[[i]], labelCex = labelCex,
labelSrt = labelSrt, labelAdj = labelAdj,
labelPos = labelPos, labelOffset = labelOffset,
labelCol = labelCol, ...)
Expand All @@ -288,9 +303,6 @@ setMethod(
matchLty = 1, matchPch = 16, ...) {
if (length(x) != 1 || length(y) != 1)
stop("'x' and 'y' have to be of length 1")
if (length(labels) & !is.function(labels))
stop("'plotSpectraMirror' supports only a function with ",
"parameter 'labels'")
if (length(col) != 2)
col <- rep(col[1], 2)
if (!length(xlim))
Expand All @@ -309,8 +321,17 @@ setMethod(
on.exit(dev.flush())
plot.new()
plot.window(xlim = xlim, ylim = ylim)
## Stop if variable modifications are used
## Will need to be removed once plotSpectra accepts variable modifications
## See issue: https://github.com/rformassspectrometry/Spectra/issues/346
if (length(labels)) {
l <- c(labels(x), labels(y))
if (is.function(labels)) {
labels <- c(labels(x), labels(y))
} else {
if (length(labels) != length(x))
stop("This Error occurs either because\n1) Annotations are not of length 2\n2) Variable modifications are not yet supported.")
}
l <- c(labels[[1]], labels[[2]])
wdths <- max(strwidth(l, cex = labelCex)) / 2
usr_lim <- par("usr")
ylim[1L] <- ylim[1L] - wdths *
Expand All @@ -319,7 +340,7 @@ setMethod(
xlim[1L] <- xlim[1L] - wdths
xlim[2L] <- xlim[2L] + wdths
plot.window(xlim = xlim, ylim = ylim, ...)
}
} else {labels <- NULL}
if (axes) {
axis(side = 1, ...)
axis(side = 2, ...)
Expand All @@ -331,7 +352,7 @@ setMethod(
x_data <- peaksData(x)[[1L]]
y_data <- peaksData(y)[[1L]]
.plot_single_spectrum(x, add = TRUE, type = type, col = col[[1L]],
labels = labels, labelCex = labelCex,
labels = labels[[1L]], labelCex = labelCex,
labelSrt = labelSrt, labelAdj = labelAdj,
labelPos = labelPos, labelOffset = labelOffset,
labelCol = labelCol, ...)
Expand All @@ -349,7 +370,7 @@ setMethod(
labelPos <- 1
labelSrt <- -1 * labelSrt
.plot_single_spectrum(y, add = TRUE, type = type, col = col[[1L]],
labels = labels, labelCex = labelCex,
labels = labels[[2L]], labelCex = labelCex,
labelSrt = labelSrt, labelAdj = labelAdj,
labelPos = labelPos, labelOffset = labelOffset,
orientation = -1, labelCol = labelCol, ...)
Expand Down