pharmaverse
General Issue: Add `AAGE`/`AAGEU` to our ADSL template
Background Information
We don't have AAGE in our ADSL template, though we provide derive_vars_aage(). It would be good to add it to showcase the function.
Definition of Done
- [ ] Call to
derive_vars_aage()added to the ADSL template
@manciniedoardo I can definitely take a look at it, if it is okay to wait until next week?
@SolveigHolmgaard absolutely, this is marked as post-1.3 release so we are not targeting this until dec 2025 (though anything after this week is ok!)
I think we would need to update the dm dataset in pharmaversesdtm first to add the birth date.
Is this something that should be added to pharmaverseraw as well?
I would expect that once it is added in pharmaversesdtm, it will be added to pharmaverseraw as well as the raw data is created from the SDTM data.
Is this something I can add to pharmaversesdtm, or should I add an issue for it in their repo?
@SolveigHolmgaard feel free to make an issue in pharmaversesdtm and self-assign that one too. Once you implement in pharmaversesdtm, you can come back to this issue and use the new dm dataset to add AAGE/AAGEU to the ADSL template 😄
Please let @Lina2689 / @Fanny-Gautier know if you don't have access to pharmaversesdtm.
If Birth Date is added to pharmaversesdtm::dm, it's important to double-check and list all {admiral} extension packages that may be impacted by this update. For example, review where dm is used, as those packages may now include this new variable by default - even though AGE may have been manually derived or Birth Date manually added. I'm thinking of {admiralpeds}, where BRTHDTC was manually added in dm_peds. This change could have an impact, so it’s worth reviewing when the time comes.
Thanks for making me aware of this! Is there somewhere we can see the dependencies? I would assume dm is used almost everywhere. I think it could be nice to have an overview of it before I start to make any changes ready.
Alternatively, could it be an idea to add a "new" dm version called dm_admiral, as done with dm_peds if I understand you correctly?
@SolveigHolmgaard it's a good question.
Luckily we store the SDTM test data for all the {admiral} packages in the centralised {pharmaversesdtm} package repo. You can see in the data_raw folder that we already have a dm.R program which essentially just loads in the DM dataset from the CDISC pilot study, and then we currently have dm_peds.R, dm_vaccine.R, dm_metabolic.R which use dm.R as a starting point. As such I would take a look at those three files and assess the impact of adding BRTHDTC to dm.R, and make edits to them as well if need be. For instance, since BRTHDTC will now be (re)derived in dm_peds.R then perhaps we'll just need to pre-emptively drop BRTHDTC from DM at the start of that script. I'd also check out the other SDTM programs in that data-raw folder just to verify BRTHDTC is not used there, but I'd be very surprised if it were.
One final thing to note is that currently {admiralneuro}'s test SDTM datasets still live in the {admiralneuro} repo (we only move test data to {pharmaversesdtm} once the first release of a package is live) so for completeness I'd also assess impact similarly for that package - see here)
@manciniedoardo Thanks a lot for the answer!
It sounds like a bit of a rabbit hole of a task, but I guess it is like that more often than not 😄 I have a small vacation next week, but I will start to look into it when I get back.
Just checking in on the progress - did we get this update completed in pharmaversesdtm? Can we update admiral now?
@Lina2689 could we add BRTHDTC to pharmaversesdtm please?
sure @bms63, created an issue in pharmaversesdtm.
I think this is almost completed by the amazing @Lina2689 !!
https://github.com/pharmaverse/pharmaversesdtm/pull/196
@SolveigHolmgaard will you make this update now or should we re-assign??
@pharmaverse/admiral and @LaurentDassencourt this is a good one!!
We just updated the pharmaversesdtm package to have BRTHDTC so be sure to install latest dev version.