Skip to content

Conversation

@nmdefries
Copy link
Collaborator

@nmdefries nmdefries commented Jul 5, 2023

Also do some light cleanup to data scripts to make sure they run and script, output file, and output data object names all match.

I've dropped the cancovid.R script since it depends on non-included raw data files, and is not exported/documented by the package. cancovid.R has been reconstructed based on code from https://github.com/zcaiElvis/Covid19Canada

@nmdefries nmdefries requested review from brookslogan and dajmcdon July 5, 2023 20:00
Copy link
Collaborator

@brookslogan brookslogan left a comment

Choose a reason for hiding this comment

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

Nice cleanup, looks good.

Only potential blocker: @dajmcdon do you want to keep cancovid somewhere?

@nmdefries
Copy link
Collaborator Author

Differences between old and new versions of the Canadian case rate dataset:

  • The old one doesn't have data for New Brunswick
  • I changed all province names to long form, for consistency (e.g. "British Columbia" instead of "BC", in the old data)
  • More time values are available for each version in the new dataset. In the old script, the version dates got misaligned from the data, and each issue was actually older than the version date label and thus had fewer recent time_values available.
  • Case rates are overall slightly different in the new version, also because of the version alignment issue.

None of these appears to be relevant to the epipredict vignette that this dataset is used in. @dajmcdon to confirm.

@nmdefries
Copy link
Collaborator Author

@dajmcdon reminder to look at the new version of the cancovid script

Copy link
Collaborator

@dajmcdon dajmcdon left a comment

Choose a reason for hiding this comment

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

@nmdefries Thanks for the reminder. A few minor things:

  1. Some enhanced docs for cancovid.R
  2. I think that the epidatr calls are different now, so we should probably update.

@nmdefries nmdefries requested a review from dajmcdon November 9, 2023 18:21
@nmdefries nmdefries merged commit ca86f03 into main Nov 9, 2023
@nmdefries nmdefries deleted the ndefries/make-data-gen-scripts-run branch November 9, 2023 19:57
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.

4 participants