fix(dataProcessPlotsPTM, groupComparisonPlotsPTM): Migrate away from deprecated functions for ggplot#120
Conversation
WalkthroughConverted ggplot2 mappings from aes_string to aes with tidy-eval .data pronoun across plotting routines, updated aesthetic references to explicit data columns, and replaced size with linewidth for geom_line. Adjusted xintercept and label mappings accordingly. Changes are confined to R/utils_dataProcessPlots.R and R/utils_groupComparisonPlots.R. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
R/utils_groupComparisonPlots.R (2)
305-315: Import rlang::.data and bump ggplot2 minimumUsing .data requires importing it from rlang, and linewidth requires ggplot2 ≥ 3.4. Add the import and raise the minimum version to avoid NOTES/errors during R CMD check.
Apply these changes:
*** R/utils_groupComparisonPlots.R @@ -#' Check input data and parameters +#' Check input data and parameters +#' @importFrom rlang .data*** DESCRIPTION @@ -Imports: - ggplot2, +Imports: + ggplot2 (>= 3.4.0), + rlang (>= 0.4.11), + ggrepelAlso applies to: 531-535, 556-559, 582-585, 625-628, 631-633, 635-637
401-404: Fix unconditional dev.off() (plotly path will error)dev.off() is called even when no PDF device was opened (isPlotly == TRUE), which can error on the null device.
Apply:
- if (address != FALSE) { - dev.off() - } + if (!isPlotly && address != FALSE) { + dev.off() + }R/utils_dataProcessPlots.R (2)
1018-1020: Fix unconditional dev.off() (TMT QC wrapper)Same device-closing bug as in volcano plots.
Apply:
- if (address != FALSE) { - dev.off() - } + if (!isPlotly && address != FALSE) { + dev.off() + }
1-1: Add rlang::.data import and bump ggplot2 minimum (package-wide)Same as the other file: .data pronoun and linewidth imply runtime/CRAN requirements.
Apply:
*** R/utils_dataProcessPlots.R @@ -#' Determine if data is label free or TMT +#' Determine if data is label free or TMT +#' @importFrom rlang .data*** DESCRIPTION @@ -Imports: - ggplot2, +Imports: + ggplot2 (>= 3.4.0), + rlang (>= 0.4.11)Also applies to: 296-302, 421-428, 721-724, 789-793, 1144-1149, 1250-1254, 1531-1534, 1590-1593
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
R/utils_dataProcessPlots.R(13 hunks)R/utils_groupComparisonPlots.R(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
R/utils_groupComparisonPlots.R (2)
305-315: Heatmap: tidy-eval aes migration looks goodSwitch to aes(.data$...) is correct and future-proof here. No functional change otherwise.
531-535: Volcano: tidy-eval + linewidth migration looks goodThe mappings and linewidth replacements are consistent across layers.
Please confirm ggrepel is listed in Imports/Suggests (geom_text_repel usage).
Also applies to: 556-559, 582-585, 625-628, 631-633, 635-637
R/utils_dataProcessPlots.R (6)
296-302: TMT profile: tidy-eval + linewidth migration looks goodMappings and linewidth usage are correct.
Also applies to: 311-311, 314-314
421-428: TMT summary profile: tidy-eval + linewidth migration looks goodConsistent with the original intent; no behavioral drift.
Also applies to: 437-437, 441-441, 453-454
721-721: QC (all, TMT): tidy-eval migration looks goodScales/annotations unchanged besides .data usage.
Also applies to: 724-724, 729-729, 732-732
789-789: QC (single, TMT): tidy-eval migration looks goodAll good here.
Also applies to: 793-793, 797-797, 801-801
1144-1149: LF profile: tidy-eval + linewidth migration looks goodLayer mappings are correct and equivalent.
Also applies to: 1158-1158, 1162-1162
1250-1254: LF summary profile: tidy-eval + linewidth migration looks goodNo issues spotted.
Also applies to: 1263-1263, 1267-1267
| ptemp = ggplot(aes(x = .data$RUN, y = .data$ABUNDANCE), | ||
| data = datafeature) + | ||
| geom_boxplot(aes_string(fill = 'CONDITION'), outlier.shape = 1, | ||
| geom_boxplot(aes(fill = .data$CONDITION), outlier.shape = 1, | ||
| outlier.size = 1.5) + | ||
| labs(title = title, x = 'MS runs') + | ||
| scale_y_continuous(yaxis.name, y.limdown, y.limup) + |
There was a problem hiding this comment.
QC (all, LF): fix limits arg and undefined text.angle
- scale_y_continuous is using positional args; limits isn’t set.
- text_angle = text.angle references an undefined symbol in this scope.
Apply:
- scale_y_continuous(yaxis.name, y.limdown, y.limup) +
+ scale_y_continuous(yaxis.name, limits = c(y.limdown, y.limup)) +
@@
- theme_msstats(type = "PROFILEPLOT", x.axis.size, y.axis.size,
- 13,
- element_rect(fill = "gray95"),
- element_text(colour = c("#00B0F6"), size = 14),
- "none", text_angle = text.angle)
+ theme_msstats(type = "PROFILEPLOT", x.axis.size, y.axis.size,
+ 13,
+ element_rect(fill = "gray95"),
+ element_text(colour = c("#00B0F6"), size = 14),
+ "none", text_angle = 0)Also applies to: 1539-1542, 1549-1549
🤖 Prompt for AI Agents
R/utils_dataProcessPlots.R lines ~1531-1536 (also apply same fixes at 1539-1542
and 1549): scale_y_continuous currently uses positional args so limits are not
set — change it to scale_y_continuous(limits = c(y.limdown, y.limup), name =
yaxis.name) to explicitly set limits and axis label; also replace the invalid
text_angle = text.angle usage with angle = text_angle (or use a defined variable
name consistently), ensuring text_angle is defined/passed into the function or
given a default so theme(element_text(angle = text_angle)) uses a valid symbol.
| ptemp = ggplot(aes(x = .data$RUN, y = .data$ABUNDANCE), | ||
| data = sub) + | ||
| geom_boxplot(aes_string(fill = 'CONDITION'), outlier.shape = 1, | ||
| geom_boxplot(aes(fill = .data$CONDITION), outlier.shape = 1, | ||
| outlier.size = 1.5) + | ||
| labs(title = protein, x = 'MS runs') + | ||
| scale_y_continuous(yaxis.name, limits = c(y.limdown, y.limup)) + | ||
| geom_vline(data = groupname.tmp[lineNameAxis != 0], |
There was a problem hiding this comment.
QC (single, LF): fix undefined text.angle
Same issue as above: text.angle isn’t defined in this function.
Apply:
- theme_msstats(type = "PROFILEPLOT", x.axis.size, y.axis.size,
- 13,
- element_rect(fill = "gray95"),
- element_text(colour = c("#00B0F6"), size = 14),
- "none", text_angle = text.angle)
+ theme_msstats(type = "PROFILEPLOT", x.axis.size, y.axis.size,
+ 13,
+ element_rect(fill = "gray95"),
+ element_text(colour = c("#00B0F6"), size = 14),
+ "none", text_angle = 0)Also applies to: 1598-1603
1 participant
Motivation and Context
MSstatsLiP build is failing because it relies on dataProcessPlotsPTM and groupComparisonPlotsPTM, which are emitting warnings for using deprecated ggplot functions.
Changes
Testing
MSstatsLiP build succeeds now
Checklist Before Requesting a Review
Summary by CodeRabbit