-
Notifications
You must be signed in to change notification settings - Fork 7
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
New admiralonco function: derive_param_confirmed_bor #24
Comments
Wait for #19 before starting |
Reminder to use filter_pd from #18 once available |
If data should be cut at start of anti-cancer therapy, a new variable could be created defined as minimum of first PD date and start of anti-cancer therapy. This variable is specified in the |
Possible Workflow
Possible Workflow for "selections taking more than one observation into account"
|
@bundfussr probably makes sense you take on this one as it's the most complex piece and your knowledge of where to re-use things from admiral core will help, and as you're involved in the discussions with Amit and Catherine around confirmation rules. one comment for the above is when you say around the "cut at start of anti-cancer therapy", we had agreed before in the ADRS workflow that this could be handled in ANL01FL, in case that simplifies anything. |
@rossfarrugia , sorry I forgot about using ANL01FL for "cut at start of anti-cancer therapy". Maybe we should provide function flagging observations up to a certain date or a certain condition. However, we may need to rethink |
@bundfussr i added a reply in that issue but for me any extra specific rules about what constitutes a PD would have to be implemented when the PD date itself gets derived, and then this is fed into |
@rossfarrugia , I started implementing the function and I wonder how we should handle the selection of the source observations. I think the initial idea was to select observations with What about replacing |
@bundfussr sure that makes sense to me - are you OK to notify the team of this change via the Slack and update all the other issues containing |
@rossfarrugia , yes, will do. |
@bundfussr not suggesting to add even more arguments to this function, but just wanted to ask what's your opinion on how best we could cover different response values like iCR / iPR when we move beyond solid tumor/RECIST 1.1 for future releases? i was wondering if simplest way could be a pre-processing step creates a temp variable with these renamed and fitting with our BOR functions expectation. This fits in with #12 on our backlog for later. |
@rossfarrugia , the example specs for non solid tumors do not contain confirmed BOR. Thus I am not sure if recoding the response values will work. For unconfirmed BOR it will not work because there are more responses (9 responses for non solid tumors versus 6 responses for solid tumors). I think it is unlikely that we can move beyond solid tumor/RECIST 1.1 without major changes of the functions we are implementing at the moment. |
Makes sense @bundfussr - we can look deeper into this after our foundational release for solid tumor/RECIST 1.1. we could always make different flavours of these functions dedicated to different indications and response criteria down the line (assuming we can find specs), as including all possibilities in one function would make it very difficult to maintain. |
@rossfarrugia @bundfussr Should the |
@sgorm123 , good question! I agree that it looks strange at the moment. For Maybe we could add a @sgorm123 , @rossfarrugia , @amitjaingsk , what do you think? |
Hmm... i'm personally hoping we can avoid adding yet more arguments in the functions as they're already getting complex. One of the first steps in https://github.com/pharmaverse/admiralonco/blob/10_adrs_vignette_template/vignettes/adrs.Rmd is to join any ADSL vars you need for the downstream derivations (e.g. |
The issue is that we do not know which ADSL variables were added in the first steps of the ADRS flow. Furthermore, the user may want to keep other variables like If we want to avoid an extra argument, we could keep all variables from the input dataset and for subjects without observations in the input dataset we could keep all variables from ADSL which are also in the input dataset. Variables which should not be populated for the new parameter or populated differently (e.g., @sgorm123 , @rossfarrugia , @amitjaingsk , what do you think? |
I do not have a strong opinion, though I would prefer not to have a new argument and go with your suggestion @rossfarrugia , but also happy to defer to others. I do feel that the As an aside, I think ideally I would prefer only the new columns and rows created in the function are returned back (then leave it up to the user to join back with what is passed), over binding back with the input dataset in the function. However, I presume this is an admiral decision, and pros and cons with both and I am sure discussed previously. |
i like your suggestion @bundfussr - i find it more logical from a user perspective as when i take a source record as input to create a new record i just expect that everything will be kept except the |
Feature Idea
derive_param_confirmed_bor: checks for best confirmed overall response from OVR (where ANL01FL=”Y”) as default
Flexibility needed:
Details: checks for confirmed best overall response from PARAMCD=”OVR” (where ANL01FL=”Y”) up to/including first PD as default and sets new parameter record with AVAL/AVALC from the source record. Set ADT as the first date where the condition is fulfilled.
BOR is set to 'NE', in the case where the subject has only AVALC = 'SD' or 'NON-CR/NON-PD' less than xxx days after the reference date from ADSL.
Note: it is important that ANL01FL=”Y” provides only the eligible records to be considered for this derivation, as explained in the ADRS vignette.
Relevant Input
Note: for source_pd a date_source() object is expected. See https://pharmaverse.github.io/admiral/reference/derive_param_tte.html as an example of how this works.
Relevant Output
New parameter: Covers CBOR (for confirmed responses) from ADRS workflow doc.
Reproducible Example/Pseudo Code
The text was updated successfully, but these errors were encountered: