-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Added README and other htmls #2
base: main
Are you sure you want to change the base?
Conversation
@bcjaeger The good news is it's working through just using GitHub Pages which I think should be fine for our purposes. The github-action I had set up was getting through half of the Rmd file and getting an error when it tried to make the targets. Other R script in the Rmd file would run fine. When we merge this branch in, we just need to remember to move change the github pages settings so that it deploys off of the main branch instead of this branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an awesome start to the guideline. Thank you! The vignettes showing targets pipelines are particularly impressive.
I had minor comments, nothing urgent.
Given that this will be relevant for MoTrPAC projects too, should we include Catherine and discuss more on Friday?
Package: guideline-targets | ||
Title: What the Package Does (One Line, Title Case) | ||
Version: 0.0.0.9000 | ||
Authors@R: | ||
person("First", "Last", , "[email protected]", role = c("aut", "cre"), | ||
comment = c(ORCID = "YOUR-ORCID-ID")) | ||
Description: What the package does (one paragraph). | ||
License: `use_mit_license()`, `use_gpl3_license()` or friends to pick a | ||
license | ||
Encoding: UTF-8 | ||
Roxygen: list(markdown = TRUE) | ||
RoxygenNote: 7.3.1 | ||
Imports: | ||
dplyr, | ||
ggplot2, | ||
gtsummary, | ||
rmarkdown, | ||
stringr, | ||
targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add your name as lead author. Can also add mine as reviewer. All the relevant details can be copied from MotrPacHumanDataPreSuspension
's DESCRIPTION
file
For Title, how about something like "Guidelines for using the targets R Package in Wake Forest Biostats and Data Science Projects"?
For Description, what do you think of "Introduces best practices for general usage of the targets package and also provides specific recommendations and examples that highlight ways to get more out of targets in collaborations with members of the WFUSM-BDS organization"?
For the license, I recommend using MIT. Just running usethis::use_mit_license()
should do the trick by placing the license file in your main directory and updating the DESCRIPTION
file for you. It's worth reading or just skimming the license to see what it says about using the code.
Each project will look slightly different depending on its purpose, many projects may include these files/folders | ||
|
||
- **R/** - includes all R programs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we restrict the R
directory to contain only R functions? It may be helpful to do this b/c everything in the R directory is usually sourced by _targets.R
, so if the content in that directory isn't just functions it could lead to unexpected stuff or just errors.
|
||
- **packages.R** - An R program to load all packages into R session. This should be sourced in _targets.R | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a bullet point for conflicts.R
? Can write something like "An R program that declares which function names will be preferred if multiple packages use the same name for different functions (e.g., dplyr::filter
versus stats::filter
"
|
||
+ `report` : these targets create a report document | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some thoughts about suffixes for ending target names? We haven't really discussed this yet, so it may be something we need to talk about before we come up with a good policy. The suffixes are also a little more tricky because there is usually a set of suffixes that apply based on the prefix.
For example, targets that start with data
may have suffixes like this:
init
: this means the data target is initialized here, and will be modified in a later stepexcluded
: this indicates exclusions have been applied for this data targetderived
: this means new variables have been added to the data (e.g., a calculated variable like eGFR)analysis
: this means the data target can be used in analyses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to punt to a later issue if we want to formalize this.
### Functions | ||
|
||
- Functions may be named with the convention of `noun_verb_detail()`. For example, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Functions may be named with the convention of `noun_verb_detail()`. For example, | |
- Functions should be named with the convention of `noun_verb_detail()`. For example, |
I resisted the temptation to use the word 'must'
Issue #1