pharmaverse / admiral

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

Refine admiral Strategy #954

Closed bundfussr closed 2 years ago

bundfussr commented 2 years ago

We updated, extended, and removed many admiral functions in the last year and we had a lot of discussions on how flexible functions and parameters should be and on the level of nesting.

To reduce the number of updates and discussions the admiral strategy should be refined.

This helps also new contributors and developers of admiral extension packages.

Proposed roadmap:

rossfarrugia commented 2 years ago

@bundfussr Here goes with trying to summarize my take on the discussions we’ve spoken about and things that could be added to the programming strategy:

@bms63 @teckla-gsk @koegerr @thomas-neitmann @kabis-ops @millerg23 - please add.

There was a lot more context and background which I can't cover all of here and don't think should go in the documentation, but some of the factors we have to consider are the industry-wide scale we're going for with admiral, ensuring user support is scaleable by enabling users to have the confidence to solve their own issues, building trust in admiral from both potential new company adopters and regulators perspective, ...and a million more good points that i didn't take notes for!

bundfussr commented 2 years ago

@rossfarrugia , thanks for the summary.

If we want to mention the term "analysis concept" in our programming strategy, we need to define it. It is not a CDISC term. I think the "analysis concept" from the Roche specs is not useful for our purposes. E.g., there are different analysis concepts for ASTDY and AENDY.

I would avoid mentioning a threshold for the number of arguments because contributors may combine two parameters into one to fulfill the requirement. But this would result in more complex parameters and decrease the usability.

I would like to add the requirement that the purpose of the function, the purpose of each parameter, the permitted values for each parameter, and the output of the function must be clear from the documentation without knowledge of the code. I think this avoids too complex functions and parameters and the black box effect.

Regarding nesting we could add that code should not be nested without reason. Reasons are

rossfarrugia commented 2 years ago

All makes sense to me. Also to the analysis concept point, maybe adding an example would help best explain here. As soon as Thomas mentioned the "study day" example it's so much clearer. Nobody would ever want to see a unique function for every single possible ---DY variable.

rossfarrugia commented 2 years ago

FYI @bundfussr i'm working through a draft of this to share. I see its really important to agree as we start to move on with package extensions and bringing lots of new developers to the table.