Documentation: Create a Standardized Roxygen Template for Developer Vignette

[bms63]

Please select a category the issue is focused on?

Developer Guides

Let us know where something needs a refresh or put your idea here!

It has been observed that our roxygen headers are not standardized across the admiral code-base. For example, some function headers have @author or @return in different places from other function headers.

In order to standardized all of our headers, we first need to create a Standard Template that shows the order of the tags and what should go in each tag. This template will live in the Developer Guide-Programming Strategy . An existing example and guidance already exists and can be updated once agreed upon structure is met.

• Standardizing the headers also means a consistent structure across the Reference Tab on the packagedown site.
• This might be good as a check in the PR checklist that the roxygen headers follow the standard template?

Proposed Structure (Updated based on Stefan's feedback)

#' Title
#'
#' Short Description (If the description consists of more than one paragraph, use the `@description` tag.)
#'
#' @param dataset Descriptive Text (order of @param tags should be order as they appear in function)
#'
.
.
#' @param an_input Descriptive Text
#'
#' @details (Deep dive of purpose, recommend workflow, documentation/links)
#'
#' @return (What is returned by this function)
#'
#' @author (original and updating authors)
#'
#' @family (This will create an additional See Also section that links to similar functions)
#'
#' @keywords (This is helpful for our packagedown site to organize the reference page)
#'  If possible, use keywords which are used in `_pgkdown.yml`.
#'  If a new keyword is required, a corresponding section needs to be added to `_pgkdown.yml`.
#'  All keywords must be lower case.
#'
#' @seealso [related_function()] (This tag is optional. It should be used to refer to related functions.)
#'
#' @export
#'
#' @examples (Use `library(pkg_name)` rather than `pgk_name::function()`)

[bms63]

@pharmaverse/admiral Hi all. Is this sufficient for standardizing the Roxygen templates?

[bundfussr]

I would use the following

#' Title
#'
#' Short Description (If the description consists of more than one paragraph, use the `@description` tag.)
#'
#' @param dataset Descriptive Text (order of @param tags should be order as they appear in function)
#'
.
.
#' @param an_input Descriptive Text
#'
#' @details (Deep dive of purpose, recommend workflow, documentation/links)
#'
#' @return (What is returned by this function)
#'
#' @author (original and updating authors)
#'
#' @keywords (This is helpful for our packagedown site to organize the reference page)
#'  If possible, use keywords which are used in `_pgkdown.yml`.
#'  If a new keyword is required, a corresponding section needs to be added to `_pgkdown.yml`.
#'  All keywords must be lower case.
#'
#' @seealso [related_function()] (This tag is optional. It should be used to refer to related functions.)
#'
#' @export
#'
#' @examples (Use `library(pkg_name)` rather than `pgk_name::function()`)

I have updated the short description and the @keywords tag. I have added the @seealso tag and moved the @return tag.

[thomas-neitmann]

It would be super cool if we could turn this into an RStudio addin .

[bms63]

It would be super cool if we could turn this into an RStudio addin .

So we have a addin for logrx package and it kills our test coverage. Maybe this admiral roxygen header and logrx addin could live together? I would imagine there will be more as we go!! pkg name - pharmains pharins rxaddins

[bundfussr]

I like the idea of having a separate addin package. Maybe we could make it customizable. So if a template file is placed somewhere in the project, it is used to generate the roxygen header.

More ideas:

• We could implement a function which extracts the keywords from _pkgdown.yml. The result could be displayed for the @keywords tag.
• We could add an addin which comments the selected code using #'.

[thomas-neitmann]

If we decide to create {admiraldev} then I would put this function there.

[mbytc1]

For standardisation, should I delete the "warn.conflicts=F" param from library loading. It seems to be used for dplyr, but not in all cases? Very rarely used for other packages

[esimms999-gsk]

From an overly-cautious, frightened programmer: An RStudio add-in? That sounds like it is changing my development environment. Would it only change the environment for that single, current project or would it change globally? Are we going to test it on multiple configurations of RStudio to ensure it will work for everyone: RStudio Server v x.xx, RStudo Desktop v y.yy, different versions of R, etc.? Will it be as easy to remove it as it is to install and with no lasting effects?

[bms63]

From an overly-cautious, frightened programmer: An RStudio add-in? That sounds like it is changing my development environment. Would it only change the environment for that single, current project or would it change globally? Are we going to test it on multiple configurations of RStudio to ensure it will work for everyone: RStudio Server v x.xx, RStudo Desktop v y.yy, different versions of R, etc.? Will it be as easy to remove it as it is to install and with no lasting effects?

I would imagine it would load just like the styler package loads in and makes an Addin for you. All the checks we currently do would cover this for base R. I'm not really aware of workflows that check RStudio builds, but I'm sure they exists.

Honestly, I think this is more of reason to create the admiraldev package and move stuff like this into it. So we don't have any trepidations about addins causing issues for users.

[bms63]

For standardisation, should I delete the "warn.conflicts=F" param from library loading. It seems to be used for dplyr, but not in all cases? Very rarely used for other packages

@mbytc1 I'm unsure what you are referring to?

[thomas-neitmann]

@mbytc1 We have the warn.conflicts = FALSE call when loading {dplyr} in examples because otherwise a warning is printed about naming conflicts which looks bad on our website. We don't have that issue with other packages so we don't bother to specify this argument there.

[bms63]

We need the @family tag in our roxygen headers.

Also, I think we should only use keywords that appear in our programming guidance document and be insistent on this. We can add to it if needed, but I side on less keywords is better from a maintenance point of view.

' @keywords (This is helpful for our packagedown site to organize the reference page)

' Use keywords which are discussed in Categorization of Functions section of programming guidance.

' If a new keyword is required, please bring up in the issue or on the slack channel.

' All keywords must be lower case.

[mbytc1]

Do I need to update the NEWS.md file for this wehn putting in a pull request? NB in the edits I have not added @family tag or changed @keywords; ive only rearranged everything to the order provided in the oriringal issue

[bms63]

This does not need a news entry as it isn't user-facing. I think this will be brought over to admiraldev

I updated outline with @family

[esimms999-gsk]

Need some mention that we do not want importFrom in the roxygen header but, rather, in admiral-project.R.

[bms63]

Is this sufficient? https://pharmaverse.github.io/admiraldev/main/articles/programming_strategy.html#r-package-dependencies

[bundfussr]

@bms63 , should this be implemented for the upcoming release? Have we agreed on the template in the description?

We have a separate issue for the addin (https://github.com/pharmaverse/admiraldev/issues/11). Thus I think this issue need to cover the update of the programming strategy only.

[bms63]

I think the default and permitted values were outstanding questions. I feel like there were a few more???

[bundfussr]

Should we remove the release Q4 label then? I don't think we find a solution for default and permitted values until next week.

[bms63]

yes i will remove it