Add sample data script and corresponding .rda files #8
Conversation
Resolves #4. This changeset generates sample data files using data from the complex-example-forecast-hub. For this first pass, the samples are generated from local files (i.e., a cloned copy of complex-example-forcast-hub). Once complex-example-forecast-hub is fully onboarded to the cloud, we'll pull from there instead.
I can take a stab at it, but you will likely have some wording revisions. Does that information go at the top of |
|
It should go in a file called |
These are descriptions are based on similar ones here: https://github.com/Infectious-Disease-Modeling-Hubs/hubEnsembles/blob/main/R/data.R They will likely require some edits, but this should be a reasonable start.
There was a problem hiding this comment.
Adding an actual/formal review to request 2 changes:
- Add documentation for the data objects. As (partially) discussed in some misc. comments, this will involve adding a
data.Rfile that documents the data objects and also runningdevtools::document()to add the documentation to the R package. - Since we did this, we've decided on two other minor updates to the target values data format. It would be nice to get those updates in this PR as well. Those changes would be made first in the example-complex-forecast-hub repository with the source data, see this issue.
@elray1 just pushed a commit for the first note, and I'm expecting to do some updates on that documentation based on your feedback For the second issue, in the spirit of higher velocity and/orsmaller changes, my vote is to get this PR merged and then tackle the issue you linked as a separate piece of work (which I'm happy to do as a fast follow) It's okay to change our minds/evolve, but I don't like keeping PRs open for long periods of time when we do so. It likely doesn't matter for |
| #' output_type_id is not relevant for every kind of output_type (for example, | ||
| #' hubs will not expect output_type_id values when the output_type is mean or median} | ||
| #' \item{value}{the model’s prediction} | ||
| #' \item{model_id}{the name of the model} |
There was a problem hiding this comment.
@elray1 question about model_id: is that a column we'd expect to see in a hub's model output data? I thought we derived it from the filename.
There was a problem hiding this comment.
It is true that when the data are sitting in a hub, the model_id is encoded in the file name. But when we collect the data into a data frame in a working R (or in the future, python) session, the model_id is added into the data. And the intent of this example object is to represent what a user might get after running collect_hub(). (Maybe we should say that in this documentation.)
There was a problem hiding this comment.
Ah, makes sense--thank you for that clarification. Just pushed a commit with that note.
|
I'm approving this PR, with these notes:
@bsweger I'll leave it to you to do something about the first point here or not and then merge this? |
Rightly or wrongly, we've decided that this repo doesn't need tests. However, the lack of them is causing a CI failure. This commit is to see what happens if we just remove the tests directory.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Resolves #4.
This changeset generates sample data files using data from the complex-example-forecast-hub. For this first pass, the samples are generated from local files (i.e., a cloned copy of complex-example-forcast-hub).
Once complex-example-forecast-hub is fully onboarded to the cloud, we'll pull from there instead.