Fix ggplot2 v4 compatibility#10225
Conversation
- Replace deprecated 'size' parameter with 'linewidth' in element_rect() and element_line() - Fix LabelClusters() color retrieval to work with ggplot2 v4 S7 objects - Extract colors from plot scales instead of built plot data - Resolves issues satijalab#10156 and satijalab#10176
Documents fixed issues, outstanding problems, and workarounds for ggplot2 v4 S7 migration
Avoid serialization of large objects in sys.calls() by extracting only function names. This fixes massive performance degradation when using do.call(SCTransform, list(...)). Resolves satijalab#10153
Restore mols.qv.threshold functionality that was removed in commit 3dad59d. - Add 'qv' column to cols.use when reading transcripts - Apply QV threshold filter after loading transcripts - Remove qv column from output after filtering Resolves satijalab#10155
Check if scale.data exists and contains features before using it with SCT normalization. Provide clear error message when scale.data is missing or empty, directing users to run ScaleData(). Resolves satijalab#10165
Replace which.max with safe_which_max that handles NaN values and edge cases. Add warning when NaN values detected in prediction scores, indicating potential issues with normalization, k.weight parameter, or data quality. Resolves satijalab#10003
Add validation to handle cases where HVFInfo returns fewer than 3 columns, which occurs when different normalization methods (RNA + SCT) are used. Provide informative warning and use available columns for plotting. Resolves satijalab#9743
There was a problem hiding this comment.
Pull request overview
This pull request addresses critical ggplot2 v4 compatibility issues and several performance/functionality bugs in Seurat. The changes ensure compatibility with ggplot2's migration from S3 to S7 object system while fixing regression bugs and performance bottlenecks.
- Updated
DarkTheme()andLabelClusters()to work with ggplot2 v4 S7 objects - Fixed severe performance regression in
Parenting()(40-100x speedup for SCTransform workflows) - Restored missing QV filtering functionality in
ReadXenium() - Added validation and error handling in
FindTransferAnchors()andTransferData() - Fixed crash in
VariableFeaturePlot()when using mixed normalization methods
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| R/visualization.R | Updates DarkTheme() to use linewidth parameter; enhances LabelClusters() to extract colors from S7 plot scales; adds column validation in VariableFeaturePlot() |
| R/utilities.R | Optimizes Parenting() to avoid serializing large objects in call stack inspection |
| R/preprocessing.R | Restores QV threshold filtering in ReadXenium() by including qv column in data loading |
| R/integration.R | Adds scale.data validation in FindTransferAnchors(); implements safe_which_max in TransferData() to handle NaN scores |
| GGPLOT2_V4_KNOWN_ISSUES.md | Documents ggplot2 v4 compatibility issues, fixes, and migration guidance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| safe_which_max <- function(x) { | ||
| if (all(is.na(x)) || all(x == 0)) { | ||
| return(1L) # Default to first class if all are NA or 0 | ||
| } | ||
| result <- which.max(x) | ||
| if (length(result) == 0) { | ||
| return(1L) # Default to first class if which.max returns empty | ||
| } | ||
| return(result) |
There was a problem hiding this comment.
The safe_which_max function returns 1L (first class) when all values are 0 or NA. However, this silent defaulting could mask data quality issues. The warning about NaN values above (lines 3638-3646) alerts users to problems, but when all scores are legitimately 0, no warning is issued. Consider adding a warning when defaulting to the first class due to all-zero or all-NA scores, similar to the NaN warning.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
Hi ! Do you have some news ? |
Fixes #10156, #10176, #10153, #10155, #10165, #10003, #9743
Changes
DarkTheme()andLabelClusters()for S7 objectsParenting()(40-100x speedup)See GGPLOT2_V4_KNOWN_ISSUES.md for details.