Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 80.20% 80.37% +0.16%
==========================================
Files 56 56
Lines 8296 8392 +96
==========================================
+ Hits 6654 6745 +91
- Misses 1642 1647 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances MoleculeRankPlot to support additional QC annotations (min/max threshold lines, category cell counts, rug distribution) and optional faceting by a group_by column, with corresponding documentation and tests.
Changes:
- Added new
MoleculeRankPlotparameters:n_umi_min_threshold,n_umi_max_threshold,highlight_cell_counts,rug, andsplit(faceting). - Extended type-check utilities to allow
assert_within_limits(..., allow_null = TRUE)for optional numeric inputs. - Updated docs, NAMESPACE imports (ggplot2
geom_rug), tests, and CHANGELOG entry.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
R/molecule_rank_plot.R |
Implements new plot features, input validation, faceting, rug, and cell-count annotations. |
R/types_check.R |
Adds allow_null support to assert_within_limits() to validate optional numeric thresholds. |
R/generics.R |
Updates MoleculeRankPlot roxygen docs for new parameters/behavior. |
R/pixelatorR-package.R |
Adds roxygen import for ggplot2::geom_rug. |
NAMESPACE |
Imports geom_rug from ggplot2. |
man/type_check_helpers.Rd |
Regenerates docs reflecting the updated assert_within_limits() signature. |
man/MoleculeRankPlot.Rd |
Regenerates docs reflecting new MoleculeRankPlot arguments and updated examples. |
tests/testthat/test-MoleculeRankPlot.R |
Adds tests covering new arguments and split/facet behavior. |
CHANGELOG.md |
Notes the new MoleculeRankPlot capabilities. |
Files not reviewed (2)
- man/MoleculeRankPlot.Rd: Generated file
- man/type_check_helpers.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
maxkarlsson
left a comment
There was a problem hiding this comment.
Looks good! Found 1 potential bug and had two comments.
| } | ||
|
|
||
| if (highlight_cell_counts) { | ||
| min_y <- max(n_umi_min_threshold - 0.2 * n_umi_min_threshold, 0) |
There was a problem hiding this comment.
We could make it simpler and just set this to n_umi_min_threshold and use vjust to control positioning (?)
There was a problem hiding this comment.
Yeah that works too, but I felt it was more boiler plate code because I had to add multiple geom_text layer. I made some AI-powered improvements which should be more robust.
Description
This PR updates
MoleculeRankPlotto enable:group_bycolumnFixes: PNA-2283
Type of change
How Has This Been Tested?
Updated tests in
tests/testthat/test-MoleculeRankPlot.RPR checklist: