Skip to content

Parameterize the articles on data version; add the attributes supplementary table - #4

Merged
lwaldron merged 2 commits into
mainfrom
reproduce-2026-11
Nov 8, 2026
Merged

lwaldron merged 2 commits into
mainfrom
reproduce-2026-11

Conversation

@lwaldron

@lwaldron lwaldron commented Nov 8, 2026

Copy link
Copy Markdown
Member

Prepares the manuscript figures and tables for re-running on the next bugphyzz release. No rendered output is included; the pkgdown site is rebuilt once the final export and Zenodo deposit exist.

  • Every article takes params$bugphyzz_version (a Zenodo DOI or devel, default devel for now) and calls importBugphyzz(version = ...). The old importBugphyzz(v = 0.5) no longer matches the function signature, and the in-article MCC/R2 filters are redundant since bugphyzzExports applies them at export (bugphyzzExports#67).
  • data_integration_and_stats also takes params$validation_tsv (path or URL of a taxPProValidation validation_summary.tsv; empty uses the installed package's copy), so Figure 2 can be rendered for an older validation version. It writes Supplementary Table 1 (sources) and a new Supplementary Table 2 (one row per attribute: type, sources, curated/IBD/ASR counts, taxa, per-value MCC or R2 at rank all) as TSVs next to the output.
  • enrichment_dbbact and inst/rmd/bsdb_signatures read Uberon from a pinned release (v2024-05-13) with ontologyIndex; bugsigdbr::getOntology() fails on its download step here. nychanes pins BugSigDB to v1.2.1 like the other articles.

Rendered locally against release v1.0.3 and devel: Figures 1 and 2, the supplementary counts figure, both supplementary tables, the gingival-plaque and NYC-HANES articles; the dbBact article is running.

Drafted with Claude Code.

…entary table

importBugphyzz(version = params$bugphyzz_version) replaces the stale v = 0.5
call and the in-article threshold filters, which the export now applies.
data_integration_and_stats also takes validation_tsv, writes Supplementary
Tables 1 and 2 as TSVs, and gains a per-attribute table with counts and
validation scores. Uberon is pinned to its May 2024 release; BugSigDB
v1.2.1 everywhere.

Co-authored-by: Claude Code <noreply@anthropic.com>
readr::read_tsv(params$validation_tsv, show_col_types = FALSE)
} else {
bugphyzz:::.validationData()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Without validation_tsv, the validation table comes from the installed bugphyzz via the internal bugphyzz:::.validationData(), not from the release named by bugphyzz_version. If you pass a DOI and leave validation_tsv empty, Figure 2 and the new Supplementary Table 2 can show MCC/R2 values from a different version than the annotation counts next to them. Nothing warns about this.

Suggested fixes:

  • Make validation_tsv required whenever bugphyzz_version != "devel", or stop() / warning() when the two are inconsistent.
  • Replace ::: with an exported accessor. Bioconductor guidelines discourage :::, and an internal rename would silently break the vignette. If no accessor exists, export one in bugphyzz.

Comment on lines +639 to +643
```{r write supp tables, include = FALSE}
## TSV copies next to the rendered article, for the manuscript supplement
write.table(supp_attr_tbl, "supplementary_table_attributes.tsv", sep = "\t", quote = FALSE, row.names = FALSE)
write.table(sources, "supplementary_table_sources.tsv", sep = "\t", quote = FALSE, row.names = FALSE)
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This chunk writes TSVs to the knit working directory. That is a side effect on the source tree when built with R CMD build/BiocCheck, and the location varies with how the article is rendered (rmarkdown::render vs pkgdown). It also uses an unnamed relative path, and supplementary_table_sources.tsv is not mentioned in the PR's table list.

Fix: take an output directory from a param (e.g. params$out_dir, defaulting to tempdir()), build the path with file.path(), and use readr::write_tsv (readr is already in Imports). Also add *.tsv outputs to .gitignore/.Rbuildignore if they can land in the repo.

Comment on lines +620 to +628
summarise(
Type = unique(Attribute_type)[1],
Unit = if ("Unit" %in% names(.x)) paste(unique(na.omit(Unit)), collapse = ";") else "",
Sources = n_distinct(Attribute_source[!is.na(Attribute_source)]),
Curated = sum(Evidence %in% c("exp", "igc", "tas", "nas")),
IBD = sum(Evidence == "tax"),
ASR = sum(Evidence == "asr"),
Total = n(),
Taxa = n_distinct(NCBI_ID)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new supplementary table has no test or check. Total is n() while Curated + IBD + ASR counts only specific Evidence codes, so any other or NA Evidence value makes the columns stop summing to Total without notice. The join to validation is by tolower(Attribute), so a mismatch gives silent NA in the validation column.

Suggested fixes:

  • Add stopifnot(with(supp_attr_tbl, all(Curated + IBD + ASR == Total))) after the table is built.
  • Assert that every row has a validation string, or document which attributes are expected to be NA.
  • Note that Sources / Type use unique(...)[1], which hides inconsistent values within an attribute.

@claude

claude Bot commented Nov 8, 2026

Copy link
Copy Markdown

Main points, most important first:

  1. Validation/version mismatch (data_integration_and_stats.Rmd). With the default empty validation_tsv, the validation table comes from the installed bugphyzz through internal :::.validationData(). It does not come from the release set by bugphyzz_version, so Figure 2 and Supplementary Table 2 can mix versions silently. Require or warn on this, and use an exported accessor rather than :::.
  2. No checks on the new Supplementary Table 2. Curated + IBD + ASR need not equal Total, and the tolower() join to validation can give silent NAs. Add stopifnot assertions.
  3. TSVs are written to the knit working directory with a hard-coded relative path. Add an output-directory param (default tempdir()) and use file.path().
  4. Unrelated to the diff lines but worth tracking: the default bugphyzz_version = "devel" makes the articles non-reproducible. Pin a DOI before the pkgdown rebuild.

… match the data version

Co-authored-by: Claude Code <noreply@anthropic.com>
@lwaldron

lwaldron commented Nov 8, 2026

Copy link
Copy Markdown
Member Author
  1. Added a warning when bugphyzz_version is not devel and validation_tsv is empty, naming the installed bugphyzz version. Kept bugphyzz:::.validationData() because bugphyzz exports no accessor for that table; worth adding there.
  2. stopifnot(): Curated + IBD + ASR equals Total for every attribute, and every attribute with ASR annotations has a validation string. The join is by lowercased name because the export lowercases attribute names.
  3. New out_dir param (default tempdir()), file.path(), readr::write_tsv(); the sources TSV is now mentioned in the description.
  4. Agreed; the default moves to the new Zenodo DOI before the pkgdown rebuild.

Drafted with Claude Code.

@lwaldron
lwaldron merged commit 471e5ac into main Nov 8, 2026
1 check passed
@lwaldron
lwaldron deleted the reproduce-2026-11 branch November 8, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant