Closes #73: add ADVS example and update ADSL example and corresponding specs by Fanny-Gautier · Pull Request #83 · pharmaverse/examples

Fanny-Gautier · 2025-01-23T15:09:11Z

Pull Request

DESCRIPTION GOES HERE

Before you submit your pull request, take a look at the following checklist. Many thanks for your contribution!

Title: Place Closes #<insert_issue_number> at the beginning of your PR title. Use the Edit button in the top-right if you need to update.
Linked Issue: Ensure the related issue is linked in the "Development Section" on the right-hand side.
First Contribution: If this is your first contribution, add yourself to the DESCRIPTION file.
Impact on Examples: If your updates impact any examples, review locally for warnings or errors in the impacted example pages.
Merge Conflicts: Developers should address any merge conflicts and merge upon successful review.
New Packages: If new packages were used, ensure they are included in the DESCRIPTION file's Imports section.
Updated Examples: If you added or updated an example, ensure it runs on the latest CRAN release versions of all packages used.
Testing Instructions: Provide instructions on how to test the code if necessary.

…se build_from_derived() function

bms63

I am so excited about this @Fanny-Gautier !!!! I will try to get to early next week.

@pharmaverse/admiral we got some new integrated examples coming online as a result of @Fanny-Gautier hard work. Anyone got time to help out with the review?

This aligns ADSL closer to admiral ADSL and introduces ADVS as another example

bms63 · 2025-01-24T11:58:01Z

adam/adsl.qmd

+  where_sep_sheet = FALSE,
+  quiet = TRUE


Could we mention why we are setting these arguments like this?

where_sep_sheet explained
quiet dropped and thus left to the default value since we do not have any warnings to mute

bms63 · 2025-01-24T11:59:58Z

adam/adsl.qmd

-                                 predecessor_only = FALSE, keep = TRUE)
+                                 ds_list = list("dm" = dm_suppdm, "suppdm" = dm_suppdm),
+                                 predecessor_only = FALSE, keep = FALSE)
 head(adsl_preds, n=10)


could we display this in a prettier way using reactable package? https://github.com/pharmaverse/admiraldiscovery/blob/v0.3.0/R/interactive.R here is what Daniel uses it in admiraldiscovery

It's not looking that great without customizing it:

Could you please suggest where I could save the customization that it's not directly in the code itself ?

to compare with head()

we could just create another folder called functions at the top-level. and store the fancy code in there. and then your code could call the reactable code from the functions folder

but yeah agree the vanilla reactable looks sad

This will be implemented later, once #84 is completed.

adam/adsl.qmd

bms63 · 2025-01-24T12:02:50Z

adam/adsl.qmd

+Now we have the base dataset, we can start to create some variables.
+There are a few options to create grouping variables and their corresponding numeric variables.
+
+Option 1: We can start with creating the subgroups using the controlled terminology, in this case `AGEGR1`.


Could we make use of how the logs section does this? This is a pretty long section

At least "Option 1" should be bold.

@bms63 Could you let me know what you expect here using logr / logrx ? Thanks

metatools option | admiral option

maybe??

As agreed with @bms63 today, it will be done later since I am lost with this request.

bms63 · 2025-01-24T12:03:18Z

adam/adsl.qmd

-Now we have sorted out what we can easily do with controlled terminology it is time to start deriving some variables. Here you could refer directly to using the `{admiral}` template and [vignette](https://pharmaverse.github.io/admiral/cran-release/articles/adsl.html) in practice, but for the purpose of this end-to-end ADaM vignette we will share a few exposure derivations from there. We derive the start and end of treatment (which requires dates to first be converted from DTC to DTM), the treatment duration, and the safety population flag.
+### Exposure derivations
+
+Now we have sorted out what we can easily do with controlled terminology it is time to start deriving some variables.


So good! I'm excited!!

adam/adsl.qmd

bms63 · 2025-01-24T12:04:55Z

adam/adsl.qmd

     condition = (EXDOSE > 0 | (EXDOSE == 0 & str_detect(EXTRT, "PLACEBO")))
   ) %>%
-   drop_unspec_vars(metacore) # This will drop any columns that aren't specified in the metacore object
+   drop_unspec_vars(metacore) # This will drop any columns that are not specified in the metacore object


ummm...should this step just go at the end or am i missing something?

rossfarrugia · 2025-01-28T09:03:17Z

this is nice package we use on blogging site that automatically creates links for functions and packages. very handy - maybe should become standard (add to checklist?)

I have no strong opinion honestly speaking. In general, I'm not a fan of extending dependencies and keep things small and simple but if this helps and it's robust enough - let's go with it!

Same here @bms63 - happy if you want to go ahead and make an issue for this one to get it added across all the blogs, if anyone has time to take it on.

bundfussr

Is it possible to add a TOC to the pages like in the admiral vignettes?

Do we need to repeat so much information from the admiral vignettes?
It's not clear to me what we want to show. Do we want to show how to create an ADSL or ADVS dataset or how admiral and other pharmaverse packages work together?
In the former case I wonder why we show this in two places (admiral vignettes and pharmaverse examples). In the later case I would reduce the number of admiral function calls and focus on those where other packages are used like checks or writing xpt files or those where other packages provide alternative functions like deriving grouping variables.

adam/adsl.qmd

bundfussr · 2025-01-28T10:28:54Z

adam/adsl.qmd

+Now we have the base dataset, we can start to create some variables.
+There are a few options to create grouping variables and their corresponding numeric variables.
+
+Option 1: We can start with creating the subgroups using the controlled terminology, in this case `AGEGR1`.


At least "Option 1" should be bold.

adam/adsl.qmd

adam/advs.qmd

bms63 · 2025-01-28T12:39:03Z

Is it possible to add a TOC to the pages like in the admiral vignettes?

@Fanny-Gautier I think this is definitely needed

Do we need to repeat so much information from the admiral vignettes? It's not clear to me what we want to show. Do we want to show how to create an ADSL or ADVS dataset or how admiral and other pharmaverse packages work together? In the former case I wonder why we show this in two places (admiral vignettes and pharmaverse examples). In the later case I would reduce the number of admiral function calls and focus on those where other packages are used like checks or writing xpt files or those where other packages provide alternative functions like deriving grouping variables.

Yes, primary goal is to show how the different packages work together. Secondary goal is to have more ready to go examples for teaching workshops.

I think expanding the script is handy especially as @Fanny-Gautier has created a more robust specification file for the two datasets - we could handwave away/hide some of the derivations and say see the full script if it is indeed too wordy. TBD!

Understood on we are repeating information here, but folks coming to this site I would think would like having the full script available to them rather than an abbreviated script and then have to go to admiral to get more derivations/information on that dataset. Maintaining alignment is something giving me anxiety.

Regarding workshop goal - for ADSL we found a lot of stuff was pretty out of date and spec was pretty barebones. This update really brings stuff up to date for us. Hopefully, if someone wants to teach another workshop on programming ADSL from a spec to xpt file it will be almost 100% ready to go.

Happy to keep discussing!! Thanks for the feedback

rossfarrugia · 2025-01-30T10:43:55Z

Hi all, once all the above review comments are resolved I'm happy for this one to be approved/merged. I only had chance to review high level but I understand the perspective of having more "workshop-ready" examples, and this is something I also hear reflected in feedback from Posit colleagues. Thanks @Fanny-Gautier for all the efforts!! 🌟

rossfarrugia · 2025-01-31T16:38:49Z

@Fanny-Gautier as per chat today please name the specs file with "safety_specs" rather than ADSL, ADVS so that i can then add ADAE later in same file

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

…suppdm) call

…create_var_from_codelist()` and show the corresponding metacore object

adam/adsl.qmd

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

Fanny-Gautier · 2025-02-25T15:48:14Z

@jimrothstein Thanks a lot for your comments. Please find below my answers

harmonize ?? You mean merging, mapping, comparing, identifying? .... 2 different things into one?

harmonized means consistent metadata/specifications object

I assume SAS people understand this, I don't

This is more a CDISC's standard: There are the main data domains (= Parent in our example e.g. DM) which contain pre-defined and standard variables. New variables which are not as per standard cannot be added to a data domain. If a user has additional data for a domain which cannot be entered into the domain using the standard SDTM variables, then a supplemental qualifier dataset must be used (SUPPDM in our example).

Another excellent description, though do not know what controlled terminology means.

Controlled Terminology is the set of codelists and valid values used with data items within CDISC-defined datasets. Controlled Terminology provides the values required for submission to FDA and PMDA in CDISC-compliant datasets. Controlled Terminology does not tell you WHAT to collect; it tells you IF you collected a particular data item, how you should submit it in your electronic dataset.

In line 102, change , (comma) to and

The function takes in the previously created {metacore} object a reference variable which is in our case AGE and the name of the sub-grouped variable which is here AGEGR1. Should it be rephrased?

…e/examples into 73_add_adam_example_ADVS_ADSL

Fanny-Gautier · 2025-03-03T14:54:52Z

All good for now (Ben's comment to compare the options using lorgx might be implemented later: @rossfarrugia the PR is ready for your review. Thanks

rossfarrugia · 2025-03-04T13:38:34Z

Thanks @Fanny-Gautier for all the efforts here! I've merged now. Once Jeff's PR merges and I add the ADAE example then I'll post in pharmaverse slack to announce to the community we now have a better set of ADaM examples.

bms63 · 2025-03-04T13:44:19Z

@Fanny-Gautier many thanks for driving this through from the R/Pharma workshop. Thank you @rossfarrugia and @bundfussr for helping with review and great discussion!! Looking forward to ADAE!

Fanny-Gautier and others added 15 commits November 22, 2024 10:31

Add files via upload

90ba141

Update ADSL_ADVS_pk_spec

dfee64b

Add ADSL_ADVS_spec.xslsx

8654f25

rename ADSL_ADVS_pk_spec.xlsx to ignore them

6991e51

add adam/advs.qmd: half completed

2899d22

complete advs.qmd

557db91

update ADSL_ADVS_spec.xlsx

d202301

add advs.R

c3ab87b

#73 metadata/ADSL_ADVS_spec.xlsx: update Origin to 'Predecessor' to u…

2f9fc64

…se build_from_derived() function

#73 upload latest specs with controlled terminology

4f600f4

#73: update ADSL example

67e7693

#73: upload latest ADSL_ADVS specs

ed49501

#73 adsl.R: remove verbose = 'message' in xportr_type() call

457ee9e

#73 DESCRIPTION: add Fanny as author

493e524

#73 Delete unnecessary specs

8e9daa6

Fanny-Gautier linked an issue Jan 23, 2025 that may be closed by this pull request

ADaM example for ADVS and update ADSL (as being used for upcoming R/Pharma workshop) #73

Closed

Fanny-Gautier requested review from bms63 and rossfarrugia January 23, 2025 15:09

Fanny-Gautier marked this pull request as draft January 23, 2025 15:37

#73 adsl.qml: rename chunk

7a02d57

bms63 reviewed Jan 23, 2025

View reviewed changes

Fanny-Gautier marked this pull request as ready for review January 24, 2025 09:42

#73: using render to create R files

3c11f7f

bms63 reviewed Jan 24, 2025

View reviewed changes

adam/adsl.qmd Outdated Show resolved Hide resolved

bms63 reviewed Jan 24, 2025

View reviewed changes

adam/adsl.qmd Show resolved Hide resolved

bms63 reviewed Jan 24, 2025

View reviewed changes

bundfussr reviewed Jan 28, 2025

View reviewed changes

Fanny-Gautier and others added 12 commits January 31, 2025 17:46

#73 Rename ADSL_ADVS_spec.xlsx to safety_specs.xlsx

eaf7722

#73: adsl.qmd, adsl.R, advs.qmd, advs.R: call safety_specs.xlsx

bc8ea6c

#73 style

b150771

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

#73 style

77cf18c

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

#73 adsl.qmd, advs.qmd: add TOC

4d0bce6

#73 adsl.qmd: swap option 1 and option 2 for the grouping variables

230faea

#73 adsl.qmd: select necessary variables within head()

e0403e0

#73 adsl.qmd: rename var_input to age

67ee570

#73 adsl.qmd: clarify ds_list = list("dm" = dm_suppdm, "suppdm" = dm_…

85a77e3

…suppdm) call

#73 advs.qmd Clarify the need of pairs of code/decode for `metatool::…

4479906

…create_var_from_codelist()` and show the corresponding metacore object

#73 adsl.qmd sentence clarification in #expovars paragraph

c916067

#73 adsl.qmd: clarify returned variables in #expovars paragraph

e5d2e5d

bundfussr reviewed Feb 25, 2025

View reviewed changes

adam/adsl.qmd Outdated Show resolved Hide resolved

adam/adsl.qmd Outdated Show resolved Hide resolved

Fanny-Gautier and others added 2 commits February 25, 2025 16:17

#73 adsl.qmd bold option 2

43585ee

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

#73 adsl.qmd bold option 3

56de7ea

Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>

bundfussr approved these changes Feb 25, 2025

View reviewed changes

Fanny-Gautier added 2 commits March 3, 2025 15:47

#73 update adsl.R and advs.R as per updated qmd files

7150548

Merge branch '73_add_adam_example_ADVS_ADSL' of github.com:pharmavers…

ad10d61

…e/examples into 73_add_adam_example_ADVS_ADSL

rossfarrugia approved these changes Mar 4, 2025

View reviewed changes

rossfarrugia merged commit ca4a508 into main Mar 4, 2025
2 checks passed

rossfarrugia deleted the 73_add_adam_example_ADVS_ADSL branch March 4, 2025 13:36

Conversation

Fanny-Gautier commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Uh oh!

bms63 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rossfarrugia commented Jan 28, 2025

Uh oh!

bundfussr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bms63 commented Jan 28, 2025

Uh oh!

rossfarrugia commented Jan 30, 2025

Uh oh!

rossfarrugia commented Jan 31, 2025

Uh oh!

Uh oh!

Uh oh!

Fanny-Gautier commented Feb 25, 2025

Uh oh!

Fanny-Gautier commented Mar 3, 2025

Uh oh!

Uh oh!

rossfarrugia commented Mar 4, 2025

Uh oh!

bms63 commented Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Fanny-Gautier commented Jan 23, 2025 •

edited

Loading