pharmaverse / admiral

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

Feature Request: Implement `derive_var_merged_exist_flag()` for multiple sources #1728

Closed bundfussr closed 10 months ago

bundfussr commented 1 year ago

Feature Idea

Implement a new function similar to derive_var_merged_exist_flag() but considering multiple source datasets (for example for the ADJ, ADJAE, and ADJME parameter in ADEX) should be implemented. The function should use source objects and the source_datasets argument like in derive_var_dthcaus().

Relevant Input

No response

Relevant Output

No response

Reproducible Example/Pseudo Code

No response

bms63 commented 1 year ago

Hey @bundfussr I was going to start asking around for someone to work on this issue?

I see you are not at backlog meeting next week. Maybe we could spend some time on Wednesday meeting giving more context to this issue?

I think the first question I have: is this intended to be a new function or an enhancement to derive_var_merged_exist_flag() the language is confusing to me in the issue.

I'm leaning towards a new function as current function has 9 arguments and is easy to understand from my perspective - adding in more args and documentation might be very confusing for users

bundfussr commented 1 year ago

is this intended to be a new function or an enhancement to derive_var_merged_exist_flag()

Definitely a new function.

bms63 commented 1 year ago

Proposal for new function name: derive_var_merged_ms_exist_flag where ms is multiple sources?

bundfussr commented 1 year ago

Proposal for new function name: derive_var_merged_ms_exist_flag where ms is multiple sources?

I am not sure. I think we need to discuss the naming of the functions in general. I have made a proposal in the admiral Functions review. There I used "multi" for multiple sources. Another option is "msrc".