Skip to content

Declare openxlsx so the pkgdown build can render all vignettes - #54

Merged
cmirzayi merged 2 commits into
mainfrom
fix/pkgdown-missing-openxlsx
Nov 9, 2026
Merged

cmirzayi merged 2 commits into
mainfrom
fix/pkgdown-missing-openxlsx

Conversation

@cmirzayi

@cmirzayi cmirzayi commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

capstoneanalysis_BearklandM.Rmd loads openxlsx in an evaluated chunk, but openxlsx was not listed in DESCRIPTION and is not a dependency of anything that is, so the pkgdown workflow (which only installs declared dependencies) could not render it. Add it to Suggests.

Also build the site on pull requests to main (deploy and Codecov steps are skipped on PRs) so vignette failures surface before merge.

Refs #50

capstoneanalysis_BearklandM.Rmd loads openxlsx in an evaluated chunk,
but openxlsx was not listed in DESCRIPTION and is not a dependency of
anything that is, so the pkgdown workflow (which only installs declared
dependencies) could not render it. Add it to Suggests.

Also build the site on pull requests to main (deploy and Codecov steps
are skipped on PRs) so vignette failures surface before merge.

Refs #50

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread DESCRIPTION
rmarkdown,
broom,
jtools,
openxlsx,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

openxlsx is now declared, but capstoneanalysis_BearklandM.Rmd (line 33) also evaluates library(boot), and boot is still undeclared. It is a recommended package that usually ships with R, so it is probably installed on CI, but R CMD check and Bioconductor will still flag it. Add boot to Suggests. Also consider bumping Date: (still 2026-06-01) along with the version.

push:
branches: [main]
pull_request:
branches: [main]

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 PR trigger runs build_site_github_pages() and the full vignette build on every PR. That step is slow because it pulls curatedMetagenomicData and bugsigdb data. It also makes GITHUB_PAT: secrets.GITHUB_TOKEN a read-only token on fork PRs, which can hit API rate limits or fail the build. Consider adding a paths: filter (vignettes/**, DESCRIPTION, _pkgdown.yml, R/**) so unrelated PRs don't pay for it.

@claude

claude Bot commented Nov 9, 2026

Copy link
Copy Markdown

Review notes, most important first:

  1. library(boot) is evaluated in capstoneanalysis_BearklandM.Rmd but boot is not declared in DESCRIPTION. Add it to Suggests, as was done for openxlsx. It is a recommended package, so CI likely works, but R CMD check and Bioconductor will flag it.
  2. The new pull_request trigger runs the full vignette build on every PR. Consider a paths: filter to limit cost and fork-token failures.
  3. Minor: Date: in DESCRIPTION was not updated with the version bump.

No exported-function documentation or test changes are involved in this PR.

- fieldworkanalysis_samara: use createTaxonTable()'s current column
  names (increased_signatures / decreased_signatures).
- WomensHealth_Pregnancy: replace hard-coded 3-colour fill scales with
  scale_fill_brewer(), since the data now has 4 sequencing types.
- healthysig: BugSigDB Study 562 (the healthy reference signatures this
  analysis relies on) no longer exists, so every chunk failed. Show the
  code without evaluating it and add a note explaining why.

Refs #50

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cmirzayi
cmirzayi merged commit b6ade41 into main Nov 9, 2026
1 check passed
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