Contributing
Source:.github/CONTRIBUTING.md
Thank you for your interest in contributing to this repo! Your contribution is highly valued. Please go through this document for guidance on how to contribute.
Programming conventions
Please follow the programming conventions to ensure a consistent programming style across the package.
Generally we follow the tidyverse style guide. Some specific conventions that deviate from this are explained below.
Functions
- Function names should be explicit and clear. Words should be separated by underscore (
snake_case
). - Functions starting with
h_
are helper functions and they should not be exported. - Functions should be well documented using
roxygen2
(even when they are not exported). - Functions must be unit tested (even when they are not exported).
Imports
In package mmrm
, we follow the following convention in package imports.
- When only a few functions from another package are needed:
- In
DESCRIPTION
, add that package intoImports
. - In
mmrm-package.R
, we add aimportFrom
with a single function from the package.- The function imported can be any random function within the package.
- Always use
package::function
style wherever you need to use the function. - Example:
stats
.
- In
- When many functions from another package are needed:
- In
DESCRIPTION
, add that package intoImports
. - In
mmrm-package.R
, we use aimport
to import every function. - Use the functions directly without prefix.
- Example:
checkmate
.
- In
Comments
- Comments should always follow sentence style.
- Comments should be as minimal as possible: Generally the code and variable names should be clear enough to not need any comments. Only use them when needed and explain the “Why” and not the “What”.
Documentation
All functions must be documented using
roxygen2
chunks, including internal functions (see also above).-
Exported objects must have a
lifecycle
badge to clarify the maturity.- Start with
"experimental"
status and consider upgrading to"stable"
once the interface has been stable for several months.
- Start with
Use
Title Style
for the title of the documentation.Always include a
@description
part with at least one sentence describing the object.-
For the arguments use the following convention:
So the type of the argument is in parentheses, followed by line break, followed by lower case half-sentence ending with a full stop.
For references to other help pages use the corresponding markdown syntax, e.g.
[function()]
to reference other functions.Exported objects must be included in the
_pkgdown.yml
file to be populated on thepkgdown
website.In vignettes, you cannot directly reference help pages but only
pkgdown
web pages. Note that this includes only exported objects. To make it look similar to the help page references, please here also usefunction()
style.mmrm_review_methods.Rmd
is a large vignette and we precompute this vignette to make the GitHub actions faster. Run the scriptvignettes/precompile.R
to regenerate the precomputed vignette and subsequently update it in GitHub. Before every release we need to run this again. Please note you need to install the package and then compile the vignette because efficiency is better after installation to provide a fair comparison.
Github conventions
When using GitHub to collaborate, the following conventions are needed:
- Github issues is for issues, feature requests, bugs. Before creating a issue, please make sure this issue has not been reported yet.
- If you are going to work on this issue, please assign yourself.
- Please create a branch in the
mmrm
repository, instead of creating forks, unless you are not yet a team member.- Branches should be associated with a GitHub issue and linked to an issue id.
- The name of a branch should be of the form:
<issue_id>_<short_discription>
.
- Add changes to the branch and push it to GitHub.
- Please use clear commit messages.
- Please keep your changes focused on the issue. If there are independent changes, please separate it into another PR linking to another issue.
- Please create a Pull Request when you think your code changes are ready:
- Functions are well documented.
- Functions have corresponding unit tests.
- Changes pass all the GitHub action checks.
- The checklist in the corresponding issue is completed.
- Address all the comments you receive.
- at least one approval is needed to merge.
Contribution tips
Development environment Set-up
The development this mmrm
package is based on the latest R version and C++ compilers. The package dependencies are the most recent versions from CRAN. We recommend that your working environment is set up in the same way. Additionally, there are some tools we recommend you to install:
-
RTools
if you work on a Windows operating system. Alternatively you can usedocker
to separate the operating system and the development system. -
GitKraken
is a very useful user interface for git including visualization of git commit graphs, file history, etc. -
lintr
will allow you to perform static code analysis. -
pre-commit
is a Python module that allows you to identify issues before you commit locally.
Issue labels
The issues are categorized with several labels:
Label name | Description |
---|---|
SPx |
SP (story points) indicate complexity, and the larger the subsequent number, the more time consuming the issue is expected to be |
priority |
Issues with this label should be completed with higher priority |
good first issue |
Good choices for new team members |
blocked |
Blocked by other issues |
bug |
Something isn’t working |
devops |
Development and Operation |
discussion |
Discussion needed |
documentation |
Improvement of documentation is needed |
duplicate |
The issue already exists |
enhancement |
New feature or request |
help wanted |
Extra attention is needed |
invalid |
This doesn’t seem right |
question |
Further information needed |
Please choose an issue based on your interest, issue complexity, and priority.
Add new unit tests
To add a new unit test, you need to first identify the test scope. Does the test fit in the scope of existing tests? If so, please modify the existing test files under tests/testthat/
folder or src/
folder, depending on whether the code to be tested is R or C++. Otherwise please create a new test file, with a name prefix of “test-”.
In each test case, use the following structure:
test_that("function_name does something as expected", {
result <- function_name(input)
expected <- hardcoded_result
expect_identical(result, expected)
})
The purpose of the test should be clearly stated first.
In the test body part, conduct the tests, e.g. use expect_identical
to check consistency, expect_error
to catch error messages, etc. The test body should not follow the same implementation logic as the package did, otherwise we may miss mistakes in implementation.
Add integration tests
Integration tests compare the results of SAS and R and assures the quality of our code. To add an integration test, you need to do the following:
- Use SAS to run an appropriate mmrm model with
proc mixed
, usingfev_data
. - Save the results in
.txt
format in thedesign/SAS/
folder. - Decide the key outputs that are needed for comparison.
- Add a unit test verifying that the R implementation of the same model has the same results (conversion may be needed).
Get started with C++
If you have no experience with C++, it is totally fine: TMB
has provided us with many high-level functionalities that is very similar to R
. Here we only list the most important things that you need to go through before you begin C++ programming.
- Semicolons. C++ use semicolons to terminate a statement. In R, we can use semicolons or line breaks to do so, but in C++, we need both semicolons and line breaks.
- Types. C++ is a strong type programming language and all objects need to be declared with type. Sometimes you can combine the declaration and definition.
-
int i = 1;
This works, as we declarei
asint
and define it to be 1. -
i = 1;
This fails, becausei
is not declared yet. -
int i; i = 1;
This works, becausei
is declared and then defined. -
int i = 1, j = 2;
This works, becausei
andj
are bothint
.
-
- Object scope. In R, objects declared inside control flows will exist outside of that enclosing brackets of the flow, while in C++, objects created within control flows will be terminated.
- in R,
if (TRUE) { a = '123' }; print(a)
is legal. - in C++,
if (1) {string a = '123'}; std::stdout << a << std::endl;
is illegal, because objecta
is terminated already. - in C++,
string a; if (1) {a = '123'}; std::stdout << a << std::endl;
is legal,a
is declared prior to if statement.
- in R,
- Polymorphism. Unlike in R, where function arguments have no explicitly defined types, in C++ the type of each argument of a function must be pre-specified.
-
Template
is a special function that works with generic argument type. We could imagine a single function that could work on arguments of arbitrary type, andtemplate
functions make this possible through separation of function logic from the argument declaration. In this way we can usetemplate
functions and avoid the need to replicate the whole code for each type.
-
With these points in mind, you are about ready to go.
Get started with TMB
In mmrm
we are not including any latent variables and so the Laplace approximation aspect of TMB
is not used. We only use the automatic differentiation part of TMB
. For detailed documentation of TMB
, visit the TMB reference.
One important feature of TMB
are the R style matrix/array calculations. This is important because we mainly use this part to conduct our calculations. See matrix_arrays.cpp for examples.
Add a new covariance structure
To add a new covariance structure, you need to do the following:
- Understand the covariance structure and add appropriate documentation in covariance structure. Create a draft pull request to invite discussion from other team members.
- Implement the covariance structure on the C++ side and the corresponding R interface.
- Add unit tests to make sure the new covariance structure is working as expected.
- Add integration tests under
design/SAS/
folder to make sure SAS and R produce similar results (within tolerance).
Add additional data
To add additional data to mmrm
, please follow the next steps:
- Make sure the data is needed.
- Document the data in
R/data.R
. - Use
save
to create anrda
data only containing this dataset. Here the functionusethis::use_data
can also be helpful.
Communications within team
There are several communication channels, please use appropriate ones.
GitHub
GitHub issues and pull requests are where implementations are discussed and reviewed. Feature requests, bugs, enhancements, technical implementations can be discussed here. When you have ideas that needs to be documented, it is better to have them in GitHub.
Slack
Slack is a messaging tool and we have the mmrm
channel under the rinpharma
space. You can put anything in the slack channel, e.g., you have completed a issue and are waiting for review, or you have some questions and don’t want to wait until the next stand-up meeting.
To join the slack channel, please make sure you have a slack account, and send the email address to any team member.