Skip to content

Fix install + dashboard rendering with bundled demo data#18

Merged
amcim merged 1 commit into
mainfrom
remotes-install
May 14, 2026
Merged

Fix install + dashboard rendering with bundled demo data#18
amcim merged 1 commit into
mainfrom
remotes-install

Conversation

@amcim
Copy link
Copy Markdown
Contributor

@amcim amcim commented May 12, 2026

Description

This PR's focus is to allow the shiny app to work on the included external data, and thus user, input, when before it was defaulting to ESKAPE pathogens. This led to the dashboards being broken and not usable

  • Moved countrycode to Imports from Suggests. This was done because it was very noticable just having a major panel giving an error and would confuse a user
  • Guarded ComplexHeatmap::draw(ht...) calls with req to deal with no data render on empty panels
  • Removed observer on the Bug/Drug feature comparison tab that attempted to hardcode ESKAPE labels
  • Fixed a crash involving the legend in makeCrossModelPerformancePlot() which appears when there is only one unique strat value
  • Populated the Model holdouts Bug/Drug selectors to the loaded data. They were previously empty so the tab was not usable

What kind of change(s) are included?

  • Feature (adds or updates new capabilities)
  • Bug fix (fixes an issue).
  • Enhancement (adds functionality).
  • Breaking change (these changes would cause existing functionality to not work as expected).

Checklist

Please ensure that all boxes are checked before indicating that this pull request is ready for review.

  • I have read and followed the CONTRIBUTING.md guidelines.
  • I have searched for existing content to ensure this is not a duplicate.
  • I have performed a self-review of these additions (including spelling, grammar, and related).
  • I have added comments to my code to help provide understanding.
  • I have added a test which covers the code changes found within this PR.
  • I have deleted all non-relevant text in this pull request template.
  • Reviewer assignment: Tag a relevant team member to review and approve the changes.

@amcim amcim self-assigned this May 12, 2026
@eboyer221
Copy link
Copy Markdown
Contributor

eboyer221 commented May 13, 2026

Some issues are fixed by previous PRs that have not yet been merged. (e.g. the conversion to plotly fixes crash in makeCrossModelPerformancePlot legend with single strat value, Populate Model holdouts Bug/Drug selectors).

With regard to amRviz/amRshiny specifically if folks could review & approve in this order:

#18— install + dashboard rendering fix from @Alex McKim
#20— package rename from amRshiny to amRviz + new visualizations

After #20 merges I will need to rebase #16 onto the new main and update any 'amrShiny' -> 'amRviz' references in the test files and then #16 could be reviewed after that process is completed.
3. #16— unit tests (review after #20 merges; will need rebase to pick up the package rename)

If those PRs are merged I can likely just close #14  without merging and open up small follow-up PRs for the minor UI changes (color palette, home page redesign, etc.) that introduced.

@eboyer221 eboyer221 self-requested a review May 14, 2026 18:14
Copy link
Copy Markdown
Contributor

@eboyer221 eboyer221 left a comment

Choose a reason for hiding this comment

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

Approving, I pulled the branch and ran locally. Confirmed the Model Holdouts Bug dropdown now populates, and the country choropleth renders correctly.

@amcim amcim merged commit 5344371 into main May 14, 2026
5 checks 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.

2 participants