add vignettes

[alkaZeltser]

In this PR I'm wrapping up the majority of pre-release documentation, namely vignettes.
I know it's a lot to read, but if reviewers could skim for typos and give some feedback on comprehensibility, it would be greatly appreciated. I also made some aesthetic choices that I'm not super certain about - for example displaying various variables. Curious to hear what you think about whether it's worth doing that or not.

I'm attaching the rendered vignette in pdf form (it looks a bit better as an html page in the browser, but GitHub won't let me attach html files), along with the rendered manual. Manual has already been reviewed but if you feel inclined to double-check for consistency, that would be awesome.

UserGuide.pdf
ApplyPolygenicScore_0.1.0.pdf

• Added User Guide Vignette
• Added data and graphics files used in Vignette
• Added infrastructure for rendering vignettes
• Switched NEWS document to markdown format; @dan-knight other lab packages don't seem to follow this format recommended by R documentation, not sure what's up with that?
• Small updates to Manual to match vignette
• Added new feature to apply.polygenic.score to run just validation code
• Added test for above

• I have read the code review guidelines and the code review best practice on GitHub check-list.

• The name of the branch is meaningful and well formatted following the standards, using [AD_username (or 5 letters of AD if AD is too long)-[brief_description_of_branch].

• I have set up or verified the branch protection rule following the github standards before opening this pull request.

• I have added the changes included in this pull request to NEWS under the next release version or unreleased, and updated the date.

• I have updated the version number in metadata.yaml and DESCRIPTION.

• Both R CMD build and R CMD check run successfully.

Closes #...

Testing Results

All tests pass

[alkaZeltser]

@rhughwhite The documentation journey continues! If you're curious, the vignette and manual in this PR accompany the README page from the previous PR.

@cychen18 As promised, this is my PGS package! It's designed to be used by people in exactly your situation! Hence, I would really love to hear what you think of the documentation. If anything is unclear please let me know. You might want to start with the README which is already part of the main repository, then check out the vignette attached to this PR.

@forbiddenpersimmon This PR also has a teeny bit of functional code change to apply.polygenic.score for your eyes.

@dan-knight I am confused about the NEWS... specifically how NEWS.md should be formatted, since other BL packages don't seem to follow style recommended by Wickham. Also am hitting a wall with troubleshooting the NOTE:

* checking package subdirectories ... NOTE
Problems with news in ‘NEWS.md’:
No news entries found.

[forbiddenpersimmon]

Changes to apply.polygenic.score LGTM!

[cychen18]

this looks like an amazing super useful tool and I can't wait to try it out for my project, thank you so much for working on this!

minor formatting inconsistencies for the README under ‘Recommended Workflow’
- you have a period after step 2-4 but not after step 1 (Convert PGS weight files to BED coordinate files)
- under step 3 ‘Apply your PGS’ you have VCf instead of VCF in the third line

one typo

• in the UserGuide.pdf you linked on page 11/17 in the last line it says ‘it is indented to predict’ rather than ‘intended’

[alkaZeltser]

this looks like an amazing super useful tool and I can't wait to try it out for my project, thank you so much for working on this!

minor formatting inconsistencies for the README under ‘Recommended Workflow’ - you have a period after step 2-4 but not after step 1 (Convert PGS weight files to BED coordinate files) - under step 3 ‘Apply your PGS’ you have VCf instead of VCF in the third line

one typo

• in the UserGuide.pdf you linked on page 11/17 in the last line it says ‘it is indented to predict’ rather than ‘intended’

Thank you Caroline! I will fix those typos asap. Do you feel like the documentation did a good job guiding you through how to get started with a PGS analysis, if you don't have any prior knowledge of what that would look like?

[cychen18]

this looks like an amazing super useful tool and I can't wait to try it out for my project, thank you so much for working on this!
minor formatting inconsistencies for the README under ‘Recommended Workflow’ - you have a period after step 2-4 but not after step 1 (Convert PGS weight files to BED coordinate files) - under step 3 ‘Apply your PGS’ you have VCf instead of VCF in the third line
one typo

• in the UserGuide.pdf you linked on page 11/17 in the last line it says ‘it is indented to predict’ rather than ‘intended’

Thank you Caroline! I will fix those typos asap. Do you feel like the documentation did a good job guiding you through how to get started with a PGS analysis, if you don't have any prior knowledge of what that would look like?

Yes! I think it broke the whole process down in an easy to follow step by step order. The various functions that you also include to make certain parts easier (and check that you have the right columns/info for that particular step) plus the linked in pages for further background on certain topics makes it very accessible.

[dan-knight]

I think updating to the NEWS.md format is a great idea. Currently, we use the old NEWS text format for our packages. There are some slight differences between our packages, but none have caused any issues with CRAN checks. My first thought is to use the new format here, and I'll propose gradually standardizing our other lab R packages to match.

[rhughwhite]

Looks good! I like the example code including simulated examples etc. Also the phenotype plots look nice, very convenient.

As for the README, I get scared off by larger blocks of text and would prefer more sub-headers and bullet points, but 💁👌

[dan-knight]

One general question/comment about visualizations. Do you think it's necessary to commit the PNGs to version control? These will be rendered in the Rmd document regardless of whether you save the files. Also, the filenames are prefixed with the date which would affect the way Git handles changes to the images. Is it possible to demo the plotting functions without exporting the files to vignettes/img? Either not exporting at all, or sending it to a temp directory with tempdir() and/or tempfile().

[alkaZeltser]

One general question/comment about visualizations. Do you think it's necessary to commit the PNGs to version control? These will be rendered in the Rmd document regardless of whether you save the files. Also, the filenames are prefixed with the date which would affect the way Git handles changes to the images. Is it possible to demo the plotting functions without exporting the files to vignettes/img? Either not exporting at all, or sending it to a temp directory with tempdir() and/or tempfile().

I don't think it's strictly necessary to version control the images. I did see that BPG maintains a handful of graphics that are used in documentation so figured it would be ok.

The annoying thing about using a temporary file and re-creating the plot every time the vignette is rendered, is that the plotting function automatically adds the date to the filename. So I would need to re-generate the filename outside of the plotting function in order to construct the path to the newly written image file for rendering it. I would need to do something like this from my unit tests. Which is fine, but I kind of hate it.

The way things are organized now, the code in the vignette to generate these plots is never actually evaluated (just displayed). The images were generated once, I saved them, and then hard-coded the paths to these files in the vignette (not displayed).

If dates are an issue, I could manually rename the example files to remove the dates and update the vignette paths?

What do you think is the best course of action?

[dan-knight]

I did see that BPG maintains a handful of graphics that are used in documentation so figured it would be ok.

BPG has a few outdated practices, especially regarding vignettes. It uses the old "Sweave" .Rnw format, which is closer to LaTeX. Rmarkdown .Rmd format has a lot of cool features that help with running code and rendering figures. I'd recommend looking at CEV's vignette as an example of this. There's no maintenance of figure images or code changes. The current version of the code runs every time the vignette is built, so results are always up to date (and no extra files!)

If dates are an issue, I could manually rename the example files to remove the dates and update the vignette paths?
What do you think is the best course of action?

I was just thinking that if the package requires a plot to be exported, you could just send it to a temp directory so that it's cleaned up automatically. That way, you can just forget about it going forward.

[alkaZeltser]

I was just thinking that if the package requires a plot to be exported, you could just send it to a temp directory so that it's cleaned up automatically. That way, you can just forget about it going forward.

All my plots are multipanels, and thus cannot be simply rendered. I must export a plot, read it, then display it inline. In addition, the dates are automatically added to the filename every time the code is run, which means the path changes every time the vignette is rendered on a different date! I've done it your way, but FYI the required overhead is that I have to redundantly generate filenames for every plot:

temp.dir <- tempdir();

# make filename for Rmarkdown that matches the filename automatically generated by plotting function
basic.density.filename <- ApplyPolygenicScore:::generate.filename(
    project.stem = 'vignette-example-basic',
    file.core = 'pgs-density',
    extension = 'png'
    );

# This function makes its own filename for the plot
create.pgs.density.plot(
    pgs.data = pgs.results.with.phenotype.analysis$pgs.output,
    output.dir = temp.dir,
    filename.prefix = 'vignette-example-basic',
    file.extension = 'png'
    )

knitr::include_graphics(file.path(temp.dir, basic.density.filename));

Five times! Ugh.

[alkaZeltser]

@dan-knight I am confused about the NEWS... specifically how NEWS.md should be formatted, since other BL packages don't seem to follow style recommended by Wickham. Also am hitting a wall with troubleshooting the NOTE:

* checking package subdirectories ... NOTE
Problems with news in ‘NEWS.md’:
No news entries found.

For the record, NEWS.md issue fixed by formatting NEWS headings in a very specific way:

# PackageName Version (Date)
- newsworthy item