changes made to the csv file

[jsaltane123]

Changes were made to the .csv and associated discovery file, as there were a few inconsistencies, typos and missing variables.
The finding were first raised with the team (Ben, Amelia and Daniel) and then further discussed during a call.
I am attaching a file with detailed description of findings per each ADaM datatable.

admiraldiscovery_issue.docx

Note that the suite was built locally to check the updated table.

[bms63]

Suggested change

	ADFACE,ADSL,EVENTFL,Derive flags the record if at least one of the event occurred within the observation period,admiralvaccine,derive_vars_event_flag,https://pharmaverse.github.io/admiralvaccine/reference/derive_vars_event_flag.html,Template Example,https://github.com/pharmaverse/admiralvaccine/blob/main/inst/templates/ad_adface.R
	ADFACE,ADSL,EVENTFL,Derive flags for the record if at least one of the event occurred within the observation period,admiralvaccine,derive_vars_event_flag,https://pharmaverse.github.io/admiralvaccine/reference/derive_vars_event_flag.html,Template Example,https://github.com/pharmaverse/admiralvaccine/blob/main/inst/templates/ad_adface.R

[bms63]

Suggested change

	ADFACE,ADSL,EVENTDFL,Derive flags the record if the event is occurred,admiralvaccine,derive_vars_event_flag,https://pharmaverse.github.io/admiralvaccine/reference/derive_vars_event_flag.html,Template Example,https://github.com/pharmaverse/admiralvaccine/blob/main/inst/templates/ad_adface.R
	ADFACE,ADSL,EVENTDFL,Derive flags for the record if the event is occurred,admiralvaccine,derive_vars_event_flag,https://pharmaverse.github.io/admiralvaccine/reference/derive_vars_event_flag.html,Template Example,https://github.com/pharmaverse/admiralvaccine/blob/main/inst/templates/ad_adface.R

[bms63]

Suggested change

	ADOE,ADSL,ANL01FL,Derive flag last result within a visit and timepoint for baseline and post-baseline records,admiral,restrict_derivation,https://pharmaverse.github.io/admiral/reference/restrict_derivation.html,Template Example,https://github.com/pharmaverse/admiralophtha/blob/main/inst/templates/ad_adoe.R
	ADOE,ADSL,ANL01FL,Derive flag for last result within a visit and timepoint for baseline and post-baseline records,admiral,restrict_derivation,https://pharmaverse.github.io/admiral/reference/restrict_derivation.html,Template Example,https://github.com/pharmaverse/admiralophtha/blob/main/inst/templates/ad_adoe.R

[bms63]

Unsure on this one - others might need to be looked at as well

[jsaltane123]

@bms63 @ddsjoberg @athenamelia Could you please advise me on this one? Is the description correct or shall it be adjusted (further from the suggested version)? This will apply to a few more ANLxxFL variables.

[bms63]

ummm... i don't think these are CDISC variables??

[jsaltane123]

It is a variable which was just moved to another row, this was not added. But it does look like SDTM EX variable

[ddsjoberg]

If this is not an ADaM variable, should we not just remove? Thanks!

[jsaltane123]

Sure, will remove it

[bms63]

Wow! This is very thorough! Thank you for doing this update. @ddsjoberg is there a standard convention you all have somewhere or something we should make? Like a simple wiki page referenced at top of file?

[jsaltane123]

Changes made on 15-Jul-2024
'additional changes following review' commit

Addressing comments (thanks a lot for the review, @bms63)

• ADEG dataset – PARAM was renamed to PARAMCD (as description states ‘ Derive a list of parameter codes’
• As imputation flags were added as separate variables, the description of corresponding date/time/datetime variables should not mention imputation flags. The following variables were adjusted:
  o ADAE – ASTDTM
  o ADEX – ASTDTM and AENDTM
  o ADPC – ASTDTM
  o ADSL – DTHDT

Additional items:

• ADPPK – ASTDT and AENDT – description was slightly changed: from ‘…from date/time’ to ‘…from datetimes’.
• Description for PARAM variable was changed to be consistent across the datasets – ‘Derive a list of parameters’ (similar to ADOE and ADIS)
• Description for ‘PARAMCD’ was changed to be consistent across the datasets – ‘Derive a list of parameter codes’ (similar to ADVS)

[bms63]

@athenamelia do you mind reviewing. I believe @ddsjoberg is away this week

[athenamelia]

Thanks @jsaltane123 for the updates! This is very thorough. I looked at the spreadsheet and the additional updates to address the comments above -- it looks great!

Minor thing:

• ADMH with variable label "Derive query variables": variable name is NA. Should we add one of SMQzzNAM, SMQzzCD, SMQzzSC, SMQzzSCN, and CQzzNAM as an example for query variable?

[athenamelia]

Inconsistency for ASTDT and AENDT description labels across ADaM. "Derive analysis start/end date" vs "Derive... from datetimes".

[jsaltane123]

Changes made on 16-Jul-2024
commit

Addressing comments (thanks a lot for the review, @athenamelia )

ADMH dataset:

• empty variable with description ' Derive query variables' was removed;
• rows for SMQ02NAM, SMQ02CD, SMQ02SC, SMQ02SCN, CQ04NAM, CQ04CD were added;

All datasets:

• ASTDT description standardized to 'Derive analysis start date'
• AENDT description standardized to 'Derive analysis end date'
• ADT description standardized to 'Derive analysis date'
• ADTM description standardized to 'Derive analysis datetime'
• ATM - 'Derive analysis time'
• ATMF - 'Derive analysis time imputation flag'
• TRT01P - 'Derive planned treatment for period 1' (there was a mistake in ADVFQ)

[athenamelia]

This is great work, Julija @jsaltane123 ! Thanks for your contribution. @ddsjoberg @bms63 I don't have further comments from my end.

[bms63]

@jsaltane123 can I add you to the admiral community team. I ping folks for help on issues in different admiral projects

[bms63]

It looks great to me. Let's wait for @ddsjoberg to come back to give final seal of approval.

Also @jsaltane123 - we just had an admiral retrospectives meeting today and a new working group will be starting in later August that will look at ADAM IG Guide and what is missing in admiral. I was thinking of the TREATCOM - Treatment Compliance variable discussion that we had last week when proposing the working group. Want to get involved!?!

[jsaltane123]

@bms63 , thanks a lot for considering me for the Admiral community team. I would be happy to join and contribute to the various projects! There are other Ascent colleagues who are also keen to contribute and will introduce them at a later stage.
The new working group focused on the ADAM IG Guide sounds particularly interesting, and I'm excited about the opportunity to help address any gaps in Admiral! :)

[ddsjoberg]

HI All, I am back! Thanks for the amazing work on this PR!

I think everything looks in order. But just to be sure, can we resolve the outstanding comments? When we resolve, can we make a note whether that suggested change was made, not made (and why)?

[ddsjoberg]

@jsaltane123 thank you again for your wonderful contribution! 🕺🏼

We just need one last step. Can you run devtools::document() and push the update? I tried to do it myself, but I am blocked from modifying your branch.

[ddsjoberg]

On second thought @jsaltane123 , I can go ahead and merge this now, then push the redocs separately. You've already done so much for this PR and it's VERY MUCH appreciated!

[jsaltane123]

@bms63 , @athenamelia and @ddsjoberg thank you so much for the opportunity to contribute and for your guidance :) I look forward to making further contributions!