Skip to content

Fix particular outstanding doc and vignette issues #217

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.

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

Closed
10 tasks done
brookslogan opened this issue Aug 23, 2022 · 3 comments
Closed
10 tasks done

Fix particular outstanding doc and vignette issues #217

brookslogan opened this issue Aug 23, 2022 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers P0 high priority

Comments

@brookslogan
Copy link
Contributor

brookslogan commented Aug 23, 2022

  • Fix "%s" in epix_fill_through_version docs.
  • "[...] typo in the readme file("functions in the epiprocess package that operate on epi_df objects begin with epix", should be "epi_archive" objects)" --- thanks to Tianyue for flagging
  • Fix as_list_col confusing doc: "Should the new column be stored as a list column?" --- Roughly, "new column" should be "computations"; maybe edit some other words.
  • Remove compactify-is-not-metadata discussion from vignette.

Copy-pasting some additional items identified by Ryan in an old Slack thread:

  • on this page, we advertise “Get the epipredict R package” but the link takes you back to epiprocess
  • same page, we should probably advertise “Get the [epidatr] R package”, and not covidcast, when the former is ready
  • on this vignette, now that somebody changed the date range (?) from when I initially wrote it, some things look funny. The blue density for the lag 10 correlation (about halfway down) is weirdly bimodal. Also the argmax lag is now 22 (last plot) and not 18, as the text claims
  • In the reference index, we should write the titles using code style, so epi_df instead of epi_df, etc., because this is now possible: Use inline markdown for article/reference titles r-lib/pkgdown#2113
    • [before/after: check that GitHub Actions pkgdown has this update or otherwise does something reasonable]
  • We should go through and make sure that no messages/warnings appear in vignettes that are not important. Like any package loading message or warning, and silly ggplot message or warning , etc. I thought I caught all of them when I wrote vignettes but I either missed some or some new ones got introduced.
  • The "Attribution" section at the bottom of each vignette page starts with "This document contains dataset ...". Let's change that to "This document contains a data set ..."
@brookslogan brookslogan added documentation Improvements or additions to documentation P1 medium priority labels Aug 23, 2022
@ryantibs
Copy link
Member

Thanks @brookslogan for organizing these! I think this should be pretty quick work, can we make this happen sometime soon? (Or you can figure out somebody new-ish to delegate this to? Thank you.)

One more thing to add. The "Attribution" section at the bottom of each vignette page starts with "This document contains dataset ...". Let's change that to "This document contains a data set ..."

@brookslogan brookslogan added P0 high priority good first issue Good for newcomers and removed P1 medium priority labels Sep 21, 2022
@brookslogan
Copy link
Contributor Author

brookslogan commented Oct 21, 2022

Regarding the correlation vignette plotting change:

  • This change looks like it was from a change in the geo_values stemming from:
    • a move to use internal data in vignettes rather than query the API each time, in order to make vignette building faster and more reliable (504f2fe), plus
    • a decision to make the sample data objects small to avoid bloating the repo and package with large data sets (and maybe also improve vignette build time?). [See Consider storing vignette input data in package #72 for context.]
  • On my machine, the build time difference is 2s vs. 11s. For context, other vignettes take around ~10s as well, and growth_rate.Rmd and compactify.Rmd are major time hogs, taking ~1min a piece. build_vignettes() takes around 3min. 10s longer for vignette building and maybe 20s longer for checks isn't going to break anything; I'm basically just going to revert back to querying the API for all geo_values.
  • Later TODO: consider making an epiprocess.data package containing larger sample data objects, including the data for this vignette.
  • Later TODO: try to make vignette building work at interactive speeds.

brookslogan added a commit that referenced this issue Feb 27, 2023
…tte-issues

Fix various documentation bugs noted in #217
@brookslogan brookslogan changed the title Fix outstanding doc and vignette issues Fix particular outstanding doc and vignette issues Oct 24, 2023
@nmdefries
Copy link
Contributor

Moved remaining issue about vignette build speed to #363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers P0 high priority
Projects
None yet
Development

No branches or pull requests

4 participants