Skip to content

Reinclusion of .max_keys = Inf in autoplot.epi_workflow#483

Open
JavierMtzRdz wants to merge 3 commits into
djm/summer-cleaningfrom
jmr/max_keys-fix
Open

Reinclusion of .max_keys = Inf in autoplot.epi_workflow#483
JavierMtzRdz wants to merge 3 commits into
djm/summer-cleaningfrom
jmr/max_keys-fix

Conversation

@JavierMtzRdz

Copy link
Copy Markdown
Contributor

Checklist

Please:

  • Make sure this PR is against "dev", not "main".
  • Request a review from one of the current epipredict main reviewers:
    dajmcdon.
  • Make sure to bump the version number in DESCRIPTION and NEWS.md.
    Always increment the patch version number (the third number), unless you are
    making a release PR from dev to main, in which case increment the minor
    version number (the second number).
  • Describe changes made in NEWS.md, making sure breaking changes
    (backwards-incompatible changes to the documented interface) are noted.
    Collect the changes under the next release number (e.g. if you are on
    0.7.2, then write your changes under the 0.8 heading).
  • Consider pinning the epiprocess version in the DESCRIPTION file if
    • You anticipate breaking changes in epiprocess soon
    • You want to co-develop features in epipredict and epiprocess

Change explanations for reviewer

This PR restores .max_keys = Inf in autoplot.epi_workflow and adds a test to ensure autoplot is working.

Magic GitHub syntax to mark associated Issue(s) as resolved when this is merged into the default branch

@JavierMtzRdz JavierMtzRdz changed the title Jmr/max keys fix Reinclusion of .max_keys = Inf in autoplot.epi_workflow Jul 2, 2026
@JavierMtzRdz JavierMtzRdz mentioned this pull request Jul 2, 2026
5 tasks
@JavierMtzRdz JavierMtzRdz marked this pull request as ready for review July 2, 2026 18:25
@JavierMtzRdz JavierMtzRdz requested a review from dajmcdon as a code owner July 2, 2026 18:25
@dajmcdon

dajmcdon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@JavierMtzRdz Thanks for catching this, I'm happy to add back since it works with epiprocess/dev. Do we have any idea which version starts to break? Currently, epipredict DESCRIPTION has epiprocess >= 0.11.2. Perhaps we also wait to merge the other until an update makes it from dev to main?

@dajmcdon

dajmcdon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

(I'd rather the README runs based on the DESCRIPTION requirements, but I don't want to also leave around potential bugs that will be annoying to find later.)

@dajmcdon

dajmcdon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

(I think I'm doing this correctly)

Locally running epiprocess 0.12.0.999, on this branch I get:

> devtools::build_readme()
ℹ Installing epipredict in temporary libraryBuilding [/Users/dajmcdon/Documents/Delphi/epipredict/README.Rmd](vscode-file://vscode-app/Applications/Positron.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html#)

Quitting from README.Rmd:323-341 [plotting_forecast]
Error in autoplot_check_viable_response_vars(object, ..., non_key_cols = non_key_cols) : 
  Can't select columns with `Inf`.
✖ Can't convert from `Inf` <double> to <integer> due to loss of precision.
Error:
! ! in callr subprocess.
Caused by error in `autoplot_check_viable_response_vars(object, ..., non_key_cols = non_key_cols)`:
! Can't select columns with `Inf`.
✖ Can't convert from `Inf` <double> to <integer> due to loss of precision.

@JavierMtzRdz

JavierMtzRdz commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@dajmcdon The new feature was added after declaring the last version epiprocess 0.12.0.999. It looks like cmu-delphi/epiprocess@dev isn't necessary, since the Remotes section installs packages from "HEAD", which is dev for epiprocess. I just tested the error with an older version of epiprocess, and it was resolved by installing epipredict, provided the session is restarted before rendering the README. Could you restart the session and let me know if that resolves the error?

> pak::pkg_install("cmu-delphi/epiprocess@main")

#> ℹ No downloads are needed
#> ✔ 1 pkg + 42 deps: kept 42 [1.7s]
> rmarkdown::render("README.Rmd")
#> 
#> 
#> processing file: README.Rmd
#> |.............................................        |  85% [plotting_forecast]     
#> 
#> Error in `autoplot_check_viable_response_vars()`:
#>   ! Can't select columns with `Inf`.
#> ✖ Can't convert from `Inf` <double> to <integer> due to loss of precision.
#> Backtrace:
#>   ▆
#> 1. ├─ggplot2::autoplot(four_week_ahead, observed_response = cases_deaths)
#> 2. ├─epipredict:::autoplot.canned_epipred(four_week_ahead, observed_response = #> cases_deaths)
#> 3. │ ├─ggplot2::autoplot(...)
#> 4. │ └─epipredict:::autoplot.epi_workflow(...)
#> 5. │   ├─ggplot2::autoplot(...)
#> 6. │   └─epiprocess:::autoplot.epi_df(...)
#> 7. │     └─epiprocess:::autoplot_check_viable_response_vars(...)
#> 8. │       └─tidyselect::eval_select(rlang::expr(c(...)), object)
#> 9. │         └─tidyselect:::eval_select_impl(...)
#> 10. │           ├─tidyselect:::with_subscript_errors(...)
#> 11. │           │ └─base::withCallingHandlers(...)
#> 12. │           └─tidyselect:::vars_select_eval(...)
#> 13. │             └─tidyselect:::walk_data_tree(expr, data_mask, context_mask)
#> 14. │               └─tidyselect:::eval_c(expr, data_mask, context_mask)
#> 15. │                 └─tidyselect:::reduce_sels(node, data_mask, context_mask, init = #> init)
#> 16. │                   └─tidyselect:::walk_data_tree(new, data_mask, context_mask)
#> 17. │                     └─tidyselect:::as_indices_sel_impl(...)
#> 18. │                       └─tidyselect:::as_indices_impl(...)
#> 19. │                         └─vctrs::vec_as_subscript(x, logical = "error", call = #> call, arg = arg)
#> 20. └─rlang::cnd_signal(x) at vctrs/R/subscript.R:39:3
#> 
#> Quitting from README.Rmd:323-341 [plotting_forecast]
#> 
#> 
> devtools::install()
#> ── pak::local_install_deps() ────────────────────────────────────────────────
#> 
#> → Will update 1 package.
#> → All 2 packages (0 B) are cached.
#> + epiprocess 0.12.0 → 0.12.0.9999 👷🏿‍♂️🔧 (GitHub: 364e390)
#> 
#> ! epiprocess is loaded in the current R session, you probably need to
#> restart R after the installation.
#> ? Do you want to continue (Y/n) y
#> ℹ No downloads are needed, 1 pkg is cached
#> ✔ Installed epiprocess 0.12.0.9999 (github::cmu-delphi/epiprocess@364e390) (45ms)
#> ✔ 109 deps: kept 96, upd 1 [5.8s]                                           
#> ── R CMD build ──────────────────────────────────────────────────────────────
#> ✔  checking for file ‘/Users/javiermtz/Library/Mobile Documents/com~apple~CloudDocs#> /code-projects/delphi/epipredict/DESCRIPTION’ ...
#> ─  preparing ‘epipredict’: (584ms)
#> ✔  checking DESCRIPTION meta-information ...
#> ─  checking for LF line-endings in source and make files and shell scripts
#> ─  checking for empty or unneeded directories
#> Removed empty directory ‘epipredict/README_cache/gfm’
#> Removed empty directory ‘epipredict/README_cache’
#> NB: this package now depends on R (>= 4.1.0)
#> WARNING: Added dependency on R >= 4.1.0 because package code uses the
#> pipe |> or function shorthand \(...) syntax added in R 4.1.0.
#> File(s) using such syntax:
#>   ‘clean_f_name.Rd’ ‘step_climate.R’ ‘step_epi_slide.Rd’
#> ─  building ‘epipredict_0.2.7.tar.gz’
#> Warning in utils::tar(filepath, pkgname, compression = compression, compression_level = 9L,  :
#> storing paths of more than 100 bytes is not portable:
#> ‘epipredict/vignettes/backtesting_cache/html/arx-kweek#> preliminaries_69c7e599d6e135b30dc1d738b4464e7d.RData’
#> Warning in utils::tar(filepath, pkgname, compression = #> , compression_level = 9L,  :
#> storing paths of more than 100 bytes is #>  portable:
#> ‘epipredict/vignettes/backtesting_cache#> html/compare_single_with_result_b7619871cc83c10c4534a6d124cbab76.RData’
#> Warning in utils::tar(filepath, pkgname, #>  = compression, compression_level = 9L,  :
#> storing paths of #>  than 100 bytes is not portable:
#> ‘epipredict#> vignettes/backtesting_cache/html#> plot_just_revisioning_374ada453235acc9c36cfe21a49f4514.RData’
#> Warning in utils::tar#> filepath, pkgname, compression = compression, compression_level = 9L,  :
#> storing paths of more than 100 bytes is not portable:
#> ‘epipredict/vignettes/backtesting_cache/html#> plot_revision_example_81283a78dfcdc6d5cc45246db83f250f.RData’
#> 
#> ── R CMD INSTALL ────────────────────────────────────────────────────────────
#> Running /Library/Frameworks/R.framework/Resources/bin/R CMD INSTALL \
#> /var/folders/nq/107726sj27n2rv3dtzgb9b680000gn/T//RtmpTGISBL/epipredict_0.2.7.tar.gz \
#> --install-tests 
#> * installing to library ‘/Library/Frameworks/R.framework/Versions/4.5-arm64/Resources#> library’
#> * installing *source* package ‘epipredict’ ...
#> ** this is package ‘epipredict’ version ‘0.2.7’
#> ** using staged installation
#> ** R
#> ** inst
#> ** tests
#> ** byte-compile and prepare package for lazy loading
#> Warning message:
#> package ‘parsnip’ was built under R version 4.5.2 
#> ** help
#> *** installing help indices
#> *** copying figures
#> ** building package indices
#> ** installing vignettes
#> ** testing if installed package can be loaded from temporary location
#> Warning: package ‘parsnip’ was built under R version 4.5.2
#> ** testing if installed package can be loaded from final location
#> Warning: package ‘parsnip’ was built under R version 4.5.2
#> ** testing if installed package keeps a record of temporary installation path
#> * DONE (epipredict)
#> Warning message:
#> In get(Info[i, 1], envir = env) :
#> internal error 1 in R_decompress1 with libdeflate
#> 
> rstudioapi::restartSession(clean = TRUE)

#> Restarting R session...

> rmarkdown::render("README.Rmd")

#> 
#> processing file: README.Rmd
#> 
#> output file: README.knit.md
#> /Applications/RStudio.app/Contents/Resources/app/quarto/bin/tools/aarch64/pandoc +RTS#> -K512m -RTS README.knit.md --to gfm-tex_math_gfm-yaml_metadata_block --from markdow#> +autolink_bare_uris+tex_math_single_backslash --output README.md --template /Librar#> /Frameworks/R.framework/Versions/4.5-arm64/Resources/library/rmarkdown/rmarkdow#> /templates/github_document/resources/default.md 
#> /Applications/RStudio.app/Contents/Resources/app/quarto/bin/tools/aarch64/pandoc +RTS#> -K512m -RTS README.md --to html4 --from gfm-tex_math_gfm --output README.html --embe#> -resources --standalone --syntax-highlighting pygments --template /Library/Frameworks/#> .framework/Versions/4.5-arm64/Resources/library/rmarkdown/rmarkdown/template#> /github_document/resources/preview.html --variable 'github-markdown-css:/Librar#> /Frameworks/R.framework/Versions/4.5-arm64/Resources/library/rmarkdown/rmarkdow#> /templates/github_document/resources/github.css' --metadata pagetitle=PREVIEW --mathjax#> 
#> 
#> Preview created: README.html
#> 
#> Output created: README.md

@JavierMtzRdz JavierMtzRdz reopened this Jul 2, 2026
@JavierMtzRdz

JavierMtzRdz commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Since epiprocess 0.12.0.9999 captures all changes before release, perhaps the version you have installed is from before the autoplot update. I believe it's safer to increase the minor version number and release epiprocess on main first as you mentioned. Working on this.

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