Skip to content

Refactor summary report generation: improve formatting and update example paths #33

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

Open
AbhirupaGhosh wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −13
Open

Refactor summary report generation: improve formatting and update example paths #33

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 20 additions & 13 deletions R/amRdataPlots.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
#'
#' @examples
#' generateSummary(
#' metadata_parquet = "results/metadata.parquet",
#' out_path = "results/"
#' metadata_parquet = "data/metadata.parquet",
#' out_path = "data/"
Comment on lines +12 to +13

@jananiravi jananiravi Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe @examples will look for input files in a different location -- inst/extdata. right, @amcim @eboyer221? So that needs to be fixed -- until then, to avoid R CMD check errors, we may need to add \dontrun{}/\donttest{}.

@eboyer221 eboyer221 Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, you're right @jananiravi. There's no example data actually included in the package (inst/extdata only has some reference files, not this kind of data), so this example won't run in an automated check.

Between \dontrun{} and \donttest{}: I'd go with \dontrun{} since it just skips the example entirely, which matches what we already do in the same situation elsewhere in the package.

@jananiravi jananiravi Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

They're planning to add examples, Emily. Also, the examples will be visible on the help page but won't run when clicked on -- that's the distinction, right?

#' )
#'
#' @export
generateSummary <- function(metadata_parquet, out_path) {
generateSummary <- function(metadata_parquet,
out_path
) {
# Little helper to apply distinct + non-empty + sorted vector
clean_distinct <- function(df, col) {
df |>
Expand Down Expand Up @@ -60,7 +62,9 @@ generateSummary <- function(metadata_parquet, out_path) {
dplyr::pull()

# Core summaries
TotalEntryCount <- metadata |> dplyr::count()
TotalEntryCount <- metadata |>
dplyr::count()

CleanEntryCount <- metadata |>
dplyr::distinct(genome.genome_id) |>
dplyr::count()
Expand Down Expand Up @@ -172,7 +176,7 @@ ResPropbyDrugClass <- drug_class_calls |>
# Header
write_new(
md_path,
sprintf("# AMR summary report for *%s*", Species_name)
sprintf("# AMR summary report for *%s*", paste(Species_name, collapse = ", "))
)

# Basic stats
Expand Down Expand Up @@ -218,18 +222,14 @@ ResPropbyDrugClass <- drug_class_calls |>
append_lines(md_path, c("## Isolation sources", "", md_tbl(SourceCount), "", ""))
append_lines(md_path, c("## Hosts", "", md_tbl(HostCount), "", ""))


# Hosts as a simple list
# if (length(Host)) {
# append_lines(md_path, c("## Hosts", "", paste0("- ", Host), "", ""))
# }
invisible(md_path)
}


#' Write all summary plots to file(s)
#'
#' Expects `metadata_parquet` to be the output of `runDataProcessing()`'s
#' `cleanData()` step (or an export of the resulting `metadata` table), since
#' Expects `metadata_parquet` to be the output of
#' `cleanMetadata()` step (or an export of the resulting `metadata` table), since

@jananiravi jananiravi Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Image

needs to be changed there or here. Metadata vs. MetaData. I would recomment Metadata. that's how it's used in retrieveMetadata

Image

#' `drug_abbr`, `drug_class`, and `num_resistant_classes` are only populated
#' after that step joins in the reference drug tables.
#'
Expand All @@ -238,9 +238,16 @@ ResPropbyDrugClass <- drug_class_calls |>
#'
#' @return Invisibly returns the path to the written PDF (all plots as
#' separate pages of one multi-page file).
#'
#' @examples
#' generatePlots(
#' metadata_parquet = "data/metadata.parquet",
#' out_path = "data/"
#' )
#' @export
generatePlots <- function(metadata_parquet,
out_path) {
out_path
) {
if (!dir.exists(out_path)) {
dir.create(out_path, showWarnings = FALSE, recursive = TRUE)
}
Expand Down