Enhance plotting functions and documentation#8
Conversation
Updated documentation for plotPRC, plotROC, plotCM, plotDensity, and plotTopFeatsVI functions. Added new functions for plotting ROC curves, confusion matrices, and density of predicted class probabilities.
epbrenner
left a comment
epbrenner
left a comment
There was a problem hiding this comment.
These work, but I'd strongly suggest that the import functions for the plots to natively include the tsv reading rather than requiring it.
Add functions to plot drug phenotype distribution, model performance, and cross-drug generalization heatmaps.
I have modified the function to input tsv. |
|
@AbhirupaGhosh @jananiravi @amcim |
| #' @return A heatmap (`ggplot2` object) showing the confusion matrix. | ||
| #' | ||
| #' @export | ||
| plotCM <- function(test_data_plus_predictions_file) { |
There was a problem hiding this comment.
plotConfusion will seem odd?
| "#0F2A5A" # very dark for ~1 | ||
| ), | ||
| values = scales::rescale(c(-1, 0, 0.85, 1)), | ||
| name = "Best MCC" |
| feat_pal <- c( | ||
| "args" = "#56B4E9", # sky blue | ||
| "cogs" = "#E69F00", # orange | ||
| "genes" = "#009E73", # bluish green | ||
| "domains" = "#F0E442", # yellow | ||
| "proteins" = "#CC79A7", # reddish purple | ||
| "struct" = "#D55E00" # vermillion | ||
| ) |
There was a problem hiding this comment.
not defined separately in a function -- so it's the same colors across plots/panels?
|
I am not approving yet, but basically some of the additions I made in PR #23 were reverted. So I am taking the time now to resolve this. I re-added much of this necessary code, including adding more runnable examples, fixed some R CMD CHECK bugs, and more. Ill write some details on approval. I will be pushing changes that hopefully will resolve CI, and then will review the plotting functions functionality. |
…eanup - Re-apply genome_drug.genome_id fix in plotDrugDist/plotDrugPerf that was reverted by the previous commit (98f416c); the column in metadata.parquet is genome_drug.genome_id, not genome.genome_id - Move ComplexHeatmap from Suggests to Imports (used unconditionally in plotCrossDrug with a runnable example) - Add .readMetadata()/.readPerfInput() helpers with clear missing-file errors; route all parquet reads in the new plot functions through them - Restore plotBaselineComparison to a tibble-direct API (consistent with sibling plot functions and the vignette) and restore antibiotic x-axis labels - Fix plotMDR to return a named list(performance, prediction) matching @return, and remove the large commented-out feature-cluster block - Add column validation to plotTopFeatsVI for its new topfeat input - Remove dead commented code and fix indentation flagged by styler
56db689 to
98f416c
Compare
|
@eboyer221 @amcim I have replaced the example files. The confusion with the metadata parsing should be solved. The easiest way to check is
because the problem was at lines 432 and 511. |
|
Hi @AbhirupaGhosh thanks for the new data files! Two quick things: I ran a quick test and confirmed that the genome.genome_id fix was spot on and now the plots all work with the new data. The one issue I see: the example-data folder is now about 113 MB, and the package's automated checks need it under ~5 MB, so that's currently causing failures. The good news is most of those big files look like duplicates - all the data is already packed inside the small Sfl_parquet.duckdb file, and the plot examples only actually use metadata.parquet. The large individual files (the gene/protein/struct ones, ~26 MB each) don't seem to be used anywhere in the package. Would it be okay if I remove those big redundant files? I'll double-check everything still runs before and after, so nothing breaks. The only thing I want to confirm with you: do you need those raw files kept for anything else - like re-generating the data later, or sharing them with other people? If not, I'll clean them up and that should bring the size back down. |
vip was archived from CRAN on 2026-07-08, so setup-r-dependencies could no longer install it and every CI job failed at the dependency step. vip::vi() is still used by extractTopFeats(). Add a Remotes entry pinning vip to its last CRAN release so pak can install it from the archive. Stopgap until vip returns to CRAN or its variable-importance use is reimplemented.
Not read by any test, vignette, or executed example; plotTopClusters (its only reader) uses a \dontrun example pointing elsewhere. Brings inst/extdata from ~11MB to ~1.3MB. Removed with the author's sign-off.
|
One problem right now is that the vignette uses the files that were in The challenge here is that @eboyer221 is correct that the files need to be under a certain size, as there is a strict error check on this. But if we include files likes In the submission to Bioconductor, the order is that we will need to push all 3 packages (EH, ML, Viz), they need to have no errors or warnings, then Lori will allow us to create the EH. From there we can update ML to just utilize EH in the vignette (Currently we have Viz set up to use I am considering having the vignette not evaluate the chunks that need data for now. This will allow R CMD CHECK to pass again, and once the EH is set up, we can have eval=TRUE again for when the reviewer looks at it |
5 participants
Updated documentation for plotPRC, plotROC, plotCM, plotDensity, and plotTopFeatsVI functions. Added new functions for plotting ROC curves, confusion matrices, and density of predicted class probabilities.
Description
What kind of change(s) are included?
Checklist
Please ensure that all boxes are checked before indicating that this pull request is ready for review.