Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Synthetic National Early Warning Score vignette - move to Quarto site blog #55

Open
tomjemmett opened this issue Jun 27, 2022 · 2 comments · May be fixed by #88
Open

Synthetic National Early Warning Score vignette - move to Quarto site blog #55

tomjemmett opened this issue Jun 27, 2022 · 2 comments · May be fixed by #88
Labels
question Further information is requested

Comments

@tomjemmett
Copy link
Member

Currently, in the synthetic news data vignette, the data is loaded directly from a url.

This is a potential issue for rendering with CRAN: occasionally file downloads can fail, which will cause the vignette to error. If this happens, CRAN can archive the package.

If the vignette is to remain as it is, this file should be directly included within the package (in the inst/ directory).

However, I think this vignette is a little out of place for this package. The other vignettes detail the source of the data, and then simple examples of how to use the data. This vignette instead explains how to generate the data, which feels out of scope for this package (there is already a blog post on the subject), and nothing about useful ways of using this data for learning purposes.
@Lextuga007
Copy link
Member

Agreed, there are also a lot of package dependencies because of the vignettes. We need simpler vignettes and these can also be linked to the NHSR courses.

The vignettes currently could be moved to the repo Demos and How to so they don't get lost. Then perhaps link to them from here or is that a similar issue with CRAN? Would it be on the README?

@Lextuga007 Lextuga007 changed the title Synthetic NEWS vignette Synthetic NEWS vignette - remove url data load Sep 28, 2024
@Lextuga007 Lextuga007 added the documentation Improvements or additions to documentation label Sep 28, 2024
@Lextuga007 Lextuga007 changed the title Synthetic NEWS vignette - remove url data load Synthetic NEWS vignette - move to Quarto site blog Sep 28, 2024
@Lextuga007 Lextuga007 removed the documentation Improvements or additions to documentation label Sep 28, 2024
@Lextuga007
Copy link
Member

Looking at the vignette it is synthesising already synthesised data so it works really well as a blog. The question though is how useful is the data itself in the package? I'm not able to create a vignette using it as it's not data I'm familiar with and it's likely it has been cleaned for statistical use as the column male is 1 or 0. I think the package is better suited to data that is closer to the original, where possible, to help with familiarity.

Question: should the data be removed?

@Lextuga007 Lextuga007 added the question Further information is requested label Sep 29, 2024
@Lextuga007 Lextuga007 changed the title Synthetic NEWS vignette - move to Quarto site blog Synthetic National Early Warning Score vignette - move to Quarto site blog Sep 29, 2024
@jasonpott jasonpott linked a pull request Oct 1, 2024 that will close this issue
Open
@Lextuga007 Lextuga007 linked a pull request Oct 1, 2024 that will close this issue
Update synthetic_news_data.Rmd #88
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update synthetic_news_data.Rmd jasonpott/NHSRdatasets
2 participants
@tomjemmett @Lextuga007