pharmaverse / admiral

ADaM in R Asset Library
https://pharmaverse.github.io/admiral
Apache License 2.0
220 stars 61 forks source link

Documentation: Break "Simple ADSL Walkthrough" into two Vignettes #2395

Closed bms63 closed 3 months ago

bms63 commented 5 months ago

Please select a category the issue is focused on?

User Guides

Background

Various discussions around the use of exprs(), the ADSL Walkthrough containing non-ADSL related discussion (https://github.com/pharmaverse/admiral/issues/2391), and 1-1 Mapping (https://github.com/pharmaverse/admiral/pull/2390) have been brought up recently . After discussion in April 3rd meeting (https://github.com/pharmaverse/admiral/discussions/2169?sort=new#discussioncomment-8929601), it was decided to break up the Simple ADSL Walkthrough into two vignettes. One is focused on ADaM code, the other on programming WoW, design, etc. Both should be user-focused and kept at high-level (Get Started!)

Definition of Done

bms63 commented 5 months ago

@Fanny-Gautier this might be great task for you or someone else from cytel, if you have the time!! Sorry to possibly steal your thunder @manciniedoardo !!

Fanny-Gautier commented 5 months ago

I agree to split the current vignette into 2 separate ones: "Simple ADSL & ADVS Walkthrough", and "Programming Concepts".

  1. Could you please confirm that the ADVS info from the "Walkthrough" vignette should be repeated like in the corresponding User Guides ? The "Simple ADSL Walkthrough" and "Creating Basic ADSL" are already very similar.
  2. What did you mean by expand the exprs() section ?
  3. Title of the 2nd vignette could be "Programming Concepts" as you suggested, or "Programming Tips" ?
bms63 commented 5 months ago

Hi @Fanny-Gautier

  1. I think the ADVS should be simplified. We want to just introduce folks to some basic functionality of adding rows. We might want to simplify the ADSL as well if it can be. They shouldn't be exact copies.
  2. I think the exprs() stuff can be handled by @manciniedoardo. Maybe you all can just work in the same branch
  3. Programming Concepts sounds good to me!! We can ask for feedback for other devs later.
manciniedoardo commented 5 months ago

@Fanny-Gautier @bms63 sounds good. For the addition of ADVS, the idea that came out of Wednesday's meeting would be to showcase the addition of a parameters - maybe mean arterial pressure (derive_param_map) or something else with derive_param_computed()