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

Closes #40 use pharmaversesdtm to host dm_peds and vs_peds #84

Merged
merged 19 commits into from
Oct 16, 2024
Merged

Closes #40 use pharmaversesdtm to host dm_peds and vs_peds #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
1d2557b
need staged dependencies to check everything still works
zdz2101 Aug 1, 2024
3c79abc
demo pharmaversesdtm works, if properly installed
zdz2101 Aug 2, 2024
45f0aaa
check if this works
zdz2101 Aug 2, 2024
4bca9ef
needs to be in import
zdz2101 Aug 2, 2024
4eed445
can it be suggests?
zdz2101 Aug 2, 2024
935eec8
Merge branch 'main' into 40-move-test-data-out-to-pharmaversesdtm@adm…
rossfarrugia Aug 23, 2024
1c4d8f4
remove pharmaversesdtm:: reference
zdz2101 Aug 23, 2024
15972ab
can't be removed yet
zdz2101 Aug 23, 2024
4969902
does this work?
zdz2101 Aug 23, 2024
fc90f51
typo
zdz2101 Aug 23, 2024
a456a5b
use :: convention
zdz2101 Sep 10, 2024
f248704
Merge branch 'main' into 40-move-test-data-out-to-pharmaversesdtm@adm…
zdz2101 Sep 10, 2024
2cd69a7
only use :: formatting
zdz2101 Sep 16, 2024
5623fe6
missed a spot
zdz2101 Sep 16, 2024
716143e
missed one
zdz2101 Sep 16, 2024
37da7e7
slight inconsistencies are fixed
zdz2101 Oct 7, 2024
fcffc5d
add news entry
zdz2101 Oct 7, 2024
6f53497
Update NEWS.md
zdz2101 Oct 9, 2024
01db875
fix error
zdz2101 Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ Imports:
Suggests:
knitr,
lubridate (>= 1.7.4),
pharmaversesdtm (>= 0.2.0),
pharmaversesdtm,
rmarkdown,
stringr (>= 1.4.0),
testthat (>= 3.0.0),
tibble
Remotes: pharmaverse/pharmaversesdtm@admiralpeds_data_migration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdz2101 this bit can be removed and you could get this and pharmaverse/pharmaversesdtm#115 ready for review now, and think should be good timing for me and @Fanny-Gautier to take a look in the coming weeks

Copy link
Collaborator Author

@zdz2101 zdz2101 Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the steps go:

  1. merge the pharmaversesdtm PR first
  2. remove this line
  3. see if this PR still works with the removal of pharmaversdtm:: as well, because I think it'll still rely on a remotes until the pharmaversesdtm package is re-released to a new version

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus Step 4) add an adsl_peds and advs_peds to pharmaverseadam. we can make an issue for this later after all these steps

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for adsl_peds and advs_peds it seems there was some ongoing discussion about what is appropriate to house in there as the datasets for that package are made from the templates in admiral see discussion from this issue

Copy link
Collaborator

@rossfarrugia rossfarrugia Aug 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they mean any admiral package templates incl extensions here, as they do have ADaMs created from ophtha, onco, vaccine templates for example at https://github.com/pharmaverse/pharmaverseadam/tree/main/data

VignetteBuilder:
knitr
Config/testthat/edition: 3
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
## Documentation

- Added CRAN installation instructions to README (#10).
- Subject based test data (e.g. DM, VS) has been migrated out of `{admiralpeds}` to `{pharmaversesdtm}` (#40)

## Various

Expand Down
14 changes: 0 additions & 14 deletions R/data.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,20 +193,6 @@
#' @source \url{https://www.cdc.gov/growthcharts/percentile_data_files.htm}
"cdc_bmiage"

#' Demographic Dataset-pediatrics
#'
#' An updated SDTM DM dataset with pediatric patients
#' @keywords datasets
#' @family datasets
"dm_peds"

#' Vital signs Dataset-pediatrics
#'
#' An updated SDTM VS dataset with anthropometric measurements for pediatric patients
#' @keywords datasets
#' @family datasets
"vs_peds"

#' Subject Level Analysis Dataset-pediatrics
#'
#' An updated ADaM ADSL dataset with pediatric patients
Expand Down
5 changes: 3 additions & 2 deletions R/derive_params_growth_age.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@
#' library(lubridate, warn.conflicts = FALSE)
#' library(rlang, warn.conflicts = FALSE)
#' library(admiral, warn.conflicts = FALSE)
#' library(pharmaversesdtm, warn.conflicts = FALSE)
#'
#' advs <- dm_peds %>%
#' advs <- pharmaversesdtm::dm_peds %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be able to remove pharmaversesdtm:: - i don't think we usually include these. i guess this was just for testing purposes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to demonstrate it works being in pharmaversesdtm if its actually in there, yes

#' select(USUBJID, BRTHDTC, SEX) %>%
#' right_join(., vs_peds, by = "USUBJID") %>%
#' right_join(., pharmaversesdtm::vs_peds, by = "USUBJID") %>%
#' mutate(
#' VSDT = ymd(VSDTC),
#' BRTHDT = ymd(BRTHDTC)
Expand Down
7 changes: 4 additions & 3 deletions R/derive_params_growth_height.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@
#' library(lubridate, warn.conflicts = FALSE)
#' library(rlang, warn.conflicts = FALSE)
#' library(admiral, warn.conflicts = FALSE)
#' library(pharmaversesdtm, warn.conflicts = FALSE)
#'
#' # derive weight for height/length only for those under 2 years old using WHO
#' # weight for length reference file
#' advs <- dm_peds %>%
#' advs <- pharmaversesdtm::dm_peds %>%
#' select(USUBJID, BRTHDTC, SEX) %>%
#' right_join(., vs_peds, by = "USUBJID") %>%
#' right_join(., pharmaversesdtm::vs_peds, by = "USUBJID") %>%
#' mutate(
#' VSDT = ymd(VSDTC),
#' BRTHDT = ymd(BRTHDTC)
Expand All @@ -132,7 +133,7 @@
#' out_unit = "days"
#' )
#'
#' heights <- vs_peds %>%
#' heights <- pharmaversesdtm::vs_peds %>%
#' filter(VSTESTCD == "HEIGHT") %>%
#' select(USUBJID, VSSTRESN, VSSTRESU, VSDTC) %>%
#' rename(
Expand Down
4 changes: 2 additions & 2 deletions data-raw/adsl_peds.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ library(stringr)
# Create a basic ADSL for pediatrics ----

# Read in input data ----
data("dm_peds")
data("ex")
dm_peds <- pharmaversesdtm::dm_peds
ex <- pharmaversesdtm::ex

# Derivations ----

Expand Down
47 changes: 0 additions & 47 deletions data-raw/dm_peds.R
View file
Open in desktop

This file was deleted.

Loading
Loading