Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing variables in the table, inconsistencies #49

Closed
jsaltane123 opened this issue Jul 5, 2024 · 13 comments
Closed

Missing variables in the table, inconsistencies #49

jsaltane123 opened this issue Jul 5, 2024 · 13 comments

Comments

@jsaltane123
Copy link
Contributor

jsaltane123 commented Jul 5, 2024

  1. There are a few variables for different ADaM datasets that are present in associated templates, but not included in the Discover Admiral Functions table:
  • ADAE dataset: ASTDY and AENDY exist in the template but not in the table
  • ADSL dataset: TRT01P, DTHCAUS and DTHDOM are in the template but not in the table
  • ADEG dataset: don't think there is derivation for ADTF in the template
  • ADVS dataset: DTYPE ="LOV" and DTYPE = "AVERAGE" exist in the template but not in the table
  • ADMH dataset: description for ASTDT states 'derive analysis start date and flag' but the flag does not seem to have been actually derived, ASTDTF is missing; similar to AENDTF.
  • ADPP dataset: TRT01A and TRT01P are in the template but not in the table
  • ADPPK dataset: TRT01A and TRT01P are in the template but not in the table; also, BMIBL, BSABL, CRCLBL and EGFRBL computations were not mentioned in the table.
  1. There is a typo in the table recording for ADCM dataset: AOCCIFL should be AOCCPFL as in the code;

  2. Consistency for recording PARAMCD / DTYPE - should it be recorded with ':' or with '="'? For example

    • for ADLBHY it is recorded as PARAMCD = "HYSLAW" (in both table and in template)
    • for ADVS it is recorded as PARAMCD:BMI in the table
    • for ADVS it is recorded as DTYPE:AVERAGE in the table
@bms63
Copy link
Contributor

bms63 commented Jul 5, 2024

oh this is fantastic feedback!! thank you @jsaltane123

@pharmaverse/admiral @pharmaverse/admiral_comm this is a fun one to update!! any takers. we can discuss in more detail.

@jsaltane123
Copy link
Contributor Author

I'd be happy to participate in the discussions and assist with the updates. I can also involve a few of my colleagues from Ascent to contribute to the process!

@bms63
Copy link
Contributor

bms63 commented Jul 5, 2024

@ddsjoberg could we give write access to @jsaltane123 to help with updates. unless you want them to fork and do a PR?

@ddsjoberg
Copy link
Collaborator

@jsaltane123 fantastic!! A contribution from you and your team will be great.

To get started, you'll want to fork the repo and make edits on your copy of the repo.

Also, I am happy to have a call with you to get started to explain where the files live that need updates.

@jsaltane123
Copy link
Contributor Author

I am very excited to contribute, @bms63 and @ddsjoberg ! I'll go ahead and fork the repo as you mentioned.

A call to get started would be very helpful. Please let me know your availability for a call, and we'll arrange a time that works for everyone.

@athenamelia
Copy link
Collaborator

@jsaltane123 thanks for your comment -- this is great! I'll work on the missing variables and keep you all posted. Thanks!

@ddsjoberg
Copy link
Collaborator

@athenamelia since you know this well, why don't you meet with @jsaltane123 to walk through the structure and they can make the updates. Then you (@athenamelia ) can review the changes before we merge?

@jsaltane123
Copy link
Contributor Author

Hi @athenamelia, @ddsjoberg, and @bms63,

Thank you all for the guidance so far. I have a detailed sheet with the proposed amendments for the ADaM datasets as discussed, as well as an updated version of the admiral-lookup-book.csv file. While creating these documents, I identified further errors that might need to be addressed.

Please let me know you still need help with updating the repository. If so, it would be great to schedule a call to go over these amendments and ensure I have all the information needed to make the updates correctly. I am quite flexible this week.

@bms63
Copy link
Contributor

bms63 commented Jul 10, 2024

HI @jsaltane123 were you able to meet with @athenamelia ?

@bms63
Copy link
Contributor

bms63 commented Jul 10, 2024

I can meet with you in the morning US-EST at 9 AM. Are you on pharamaverse slack?

@jsaltane123
Copy link
Contributor Author

admiraldiscovery_issue.docx
Many thanks for arranging the meeting today! As agreed, I am adding a file with comments for every ADaM dataset mentioned in admiraldiscovery.

@bms63
Copy link
Contributor

bms63 commented Jul 11, 2024

admiraldiscovery_issue.docx Many thanks for arranging the meeting today! As agreed, I am adding a file with comments for every ADaM dataset mentioned in admiraldiscovery.

Many thanks for working on this @jsaltane123 !!

@ddsjoberg
Copy link
Collaborator

Added in #50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants