Closes #12 Add PK ADaM templates #15
Conversation
|
What do you think about ignoring the xpt, rds and csv files? Unless the user needs access to them? Just could ballon the size of the repo as we add more examples. Changed a few pieces in the homepage |
@jeffreyad any comments? from my perspective i find the xls useful as people will value seeing the example spec but i think we could avoid showing how to write the rds and xpt at the end. eg see the end of https://pharmaverse.github.io/examples/adam/ADSL.html where it shows the function that could create the xpt but we dont call it in the example. |
There was a problem hiding this comment.
@jeffreyad some comments from my review so far. my general comment though is that i think here we repeat a lot of whats in the admiral vignette https://pharmaverse.github.io/admiral/cran-release/articles/pk_adnca.html and templates. take a look at https://pharmaverse.github.io/examples/adam/ADSL.html in comparison where the focus is more on showing how admiral can be used in conjunction with the other packages and then the admiral bits themselves are minimal as people can just read the admiral vignette. my general concern with your approach is the level of maintenance required as we'd need to keep twice the number of full templates and instruction guides maintained (one for only admiral and one with the other packages added). what do you think here? i dont really have any masterplan here but in my head i was thinking not to repeat things too much here if they already exist in the individual package sites.
adam/adpc.qmd
Outdated
|
|
||
| | ADaM | Sample Code | | ||
| |--------------------|----------------------------------------------------| | ||
| | ADPC | [ad_adpc_spec.R](https://github.com/pharmaverse/e2e_pk/blob/main/ad_adpc_spec.R){target="_blank"} | |
There was a problem hiding this comment.
should all links reference this repo instead now? as i guess the e2e_pk one could be superseded by this now at some stage
There was a problem hiding this comment.
also do we need to give a template here? as to me this is extra maintenance with the templates already available from admiral
|
Also @jeffreyad 1 more comment - maybe add some |
Hi @rossfarrugia thanks for reviewing! I'll take a look |
I think this is fine to cut out out a lot of the admiral discussion and focus more on how the packages work together. |
I'm sort of interested in using reactable or DT to render the tables - the outputs look a little sad right now :( Maybe we could add this to the backlog for a later update?? |
|
Hey @rossfarrugia I think @jeffreyad has made all the required changes. Can we merge to main? |
|
@jeffreyad i spotted one of the threads has reply "Still need to make this change" so is this one ready for re-review or are you still actively updating? @bms63 happy to take another look once Jeff confirms and then hopefully get this one merged and live on the site. |
@rossfarrugia thanks, this update is complete. |
|
@jeffreyad examples are live on the site now - thanks for all the efforts! i made a couple of minor commits above for small fixes. please take a look and let me know if you see any further updates needed. |
No description provided.