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

Separate test-derive_date_vars.R into its inherent separate parts #2340

Closed
Tracked by #2337
zdz2101 opened this issue Feb 20, 2024 · 0 comments · Fixed by #2341
Closed
Tracked by #2337

Separate test-derive_date_vars.R into its inherent separate parts #2340

zdz2101 opened this issue Feb 20, 2024 · 0 comments · Fixed by #2341
Assignees
@zdz2101
Labels
documentation Improvements or additions to documentation

Comments

@zdz2101
Copy link
Collaborator

zdz2101 commented Feb 20, 2024 •
edited
Loading

From #2339, apply similar logic to testthat files
derive_date_vars.R breaks the convention of how we usually format our files, it is difficult to navigate, would be better if it was broken down into its inherent two component parts: derive_vars_dt() and derive_vars_dtm()

Defintion of done:

  1. Split derive_date_vars,R into derive_vars_dt.R, derive_vars_dtm.R date_vars_utils.R
  2. derive_vars_dt.R would contain derive_vars_dt() convert_dtc_to_dt() impute_dtc_dt() restricted_imputed_dtc_dt() and compute_dtf()
  3. derive_vars_dtm.R would contain derive_vars_dtm() get_partial_datetime() convert_dtc_to_dtm() impute_dtc_dtm() restricted_imputed_dtc_dtm() and compute_tmf()
  4. date_vars_utils.R would contain dtm_level() dt_level() get_imputation_target_date() get_imputation_target_time() convert_date_to_dtm()`
@zdz2101 zdz2101 mentioned this issue Feb 20, 2024
Closed
6 tasks
@zdz2101 zdz2101 changed the title after separating the actual function code, apply the same splits to the testthat file Apply the same splits to the testthat file for derive_date_vars() stuff Feb 20, 2024
@zdz2101 zdz2101 added the documentation Improvements or additions to documentation label Feb 20, 2024
@zdz2101 zdz2101 changed the title Apply the same splits to the testthat file for derive_date_vars() stuff Separate test-derive_date_vars.R into its inherent separate parts Feb 20, 2024
@bms63 bms63 mentioned this issue Feb 22, 2024
Merged
15 tasks
@zdz2101 zdz2101 linked a pull request Feb 23, 2024 that will close this issue
Closes #2339 split derive_date_vars.R file into inherent component parts #2341
Merged
15 tasks
@zdz2101 zdz2101 self-assigned this Feb 23, 2024
bms63 added a commit that referenced this issue Feb 26, 2024
@bms63
chore: #2340 #2339 Update NEWS.md
497eda3
bms63 added a commit that referenced this issue Feb 26, 2024
@bms63
chore: #2339 #2340 Update WORDLIST
3b5c5bf
bms63 added a commit that referenced this issue Feb 26, 2024
@zdz2101 @bms63
Closes #2339 split derive_date_vars.R file into inherent component …
a930ed1 
…parts (#2341)

* initial file split

* chore: #2339 styler

* roxygen fix

* adopt feedback

* split out testthat files

* styler and move file to where appropriate

* chore: #2340 #2339 Update NEWS.md

* chore: #2339 #2340 Update WORDLIST

---------

Co-authored-by: Ben Straub <ben.x.straub@gsk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zdz2101 zdz2101

Labels
documentation Improvements or additions to documentation
Projects
admiral (sdtm/adam, dev, ci, template, core)
Status: Archive
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Closes #2339 split derive_date_vars.R file into inherent component parts pharmaverse/admiral
1 participant
@zdz2101