Request to close 233 - iRecist vignette initial draft #265
Conversation
|
For a rendered version of the vignette see https://bundfussr.github.io/admiral/233_iRECIST_vignette/articles/irecist.html. |
vignettes/irecist.Rmd
Outdated
| This article describes creating an `ADRS` ADaM with oncology endpoint parameters | ||
| based on iRECIST. It shows a similar way of deriving the endpoints presented in | ||
| [Creating ADRS (Including Non-standard Endpoints)](adrs.html). Most of the endpoints | ||
| are derived by calling `admiral::derive_extreme_event()`. Thus the examples in this |
There was a problem hiding this comment.
I would remove the sentence "Thus...". I would not use iRecist as a starting point for Recist 1.1
There was a problem hiding this comment.
I agree , also not sure if at this point we want to bring IMWG in this vignette. I think in this section we should mention that we followed std. iRECIST guidelines as per the link below.
| **Note**: *All examples assume CDISC SDTM and/or ADaM format as input | ||
| unless otherwise specified.* | ||
|
|
||
| # Programming Workflow |
There was a problem hiding this comment.
In adrs.html (Creating ADRS (Including Non-standard Endpoints) there are parameters that are not included in the iRecist vignette. It may be worth mention that if we want to add them, they are described in Recist 1.1 vignette.
BCP Best Overall Response of CR/PR by Investigator (confirmation not required)
CBCP Best Confirmed Overall Response of CR/PR by Investigator
A1BOR Best Overall Response by Investigator (confirmation not required) - Recist 1.1 adjusted for NED at Baseline
ACCB Alternative Confirmed Clinical Benefit by Investigator
OVRB Overall Response by BICR
DEATH Death
LSTA Last Disease Assessment by Investigator
MDIS Measurable Disease at Baseline by Investigator
vignettes/irecist.Rmd
Outdated
| arg == "iPR" ~ 7, | ||
| arg == "iSD" ~ 6, | ||
| arg == "NON-iCR/NON-iUPD" ~ 5, | ||
| arg == "iCPD" ~ 4, |
There was a problem hiding this comment.
As we are ordering from best to worst answer, iCPD should be after iUPD
vignettes/irecist.Rmd
Outdated
| exist_flag = AVALC, | ||
| set_values_to = exprs( | ||
| PARAMCD = "ICPD", | ||
| PARAM = "Immune Confirmation of Disease Progression by Investigator", |
There was a problem hiding this comment.
Maybe "Confirmed" instead of "Confirmation of", as we have in Recist 1.1, so my proposal here is "Confirmed Disease Progression by Investigator"?
vignettes/irecist.Rmd
Outdated
| ```{r} | ||
| icb_y <- event( | ||
| description = paste( | ||
| "Define iCR, iPR, iSD, or NON-iCR/NON-iPD occuring at least 42 days after", |
vignettes/irecist.Rmd
Outdated
| ovr = ovr, | ||
| adsl = adsl | ||
| ), | ||
| events = list(ibor_icr, ibor_ipr, ibor_isd, ibor_non_icripd, ibor_icpd, ibor_iupd, ibor_ne, no_data_missing), |
There was a problem hiding this comment.
It seems to me that we should not differentiate the progression between iUPD and iCPD. Instead, a single iPD value with the date when the iUPD was first identified. See slides 66, 69 and 72.
If so, a change is also necessary for the ICPD parameter.
There was a problem hiding this comment.
@starosto I am not sure why the slides #66 mentioned PD as iBOR, we don't have PD as iRECIST response category. @vinhn23 @bundfussr For iBOR( unconfirmed) we need to think as this event list which use min. date value and after pick iUPD over iCPD.
Based on my discussion internally on this : we should only be assigning iUPD as BOR when a) no better response assessment exists, and b) when iCPD does not subsequently occur. When (any number of) iUPD TPRs are followed by iCPD, we should be considering them as a single block of confirmed PD for the purposes of deriving BOR and date of PD.
I checked the iBOR for 701-1028 , iBOR looks as I expected..
There was a problem hiding this comment.
I have some experience with PCWG3 criteria (Bone scans + Recist 1.1). In the bone scans uPD (unconfirmed progression) was also defined, but in the instructions it was written that as BOR we do not differentiate between uPD and PD and the Investigator filling out the BOR form (RSTESTCD=BESTRESP) is obliged to change uPD to PD.
On the other hand, there is an article on the EORTC website https://recist.eortc.org/recist/wp-content/uploads/sites/4/2017/03/Manuscript_IRECIST_Lancet-Oncology_Seymour-et-al_revision_FINAL_clean_nov25.pdf and on page 29 it shows iUPD and iCPD as iBOR categories.
I will investigate internally what expectations would be regarding progression and iBOR.
For sure the date of progression needs to be corrected with iCPD BOR and I still wonder about PARAMCD=ICPD, if it is sufficient, because it only indicates two patients with progression, while patients 01-701-1015 or 01-701-1115 are also suspicious.
There was a problem hiding this comment.
will update with new code
vignettes/irecist.Rmd
Outdated
| ```{r} | ||
| icbor_ipr <- event_joined( | ||
| description = paste( | ||
| "Define partial response (iPR) for confirmed best overall response (ICBOR) as", |
vignettes/irecist.Rmd
Outdated
| ignore_event_order = TRUE, | ||
| set_values_to = exprs( | ||
| PARAMCD = "ICRSP", | ||
| PARAM = "Immune Response by Investigator (confirmation required)", |
There was a problem hiding this comment.
I would keep consistency with adrs.html and would assign PARAM = "Confirmed Response by Investigator"
| ) | ||
| ``` | ||
|
|
||
| ## Define Events |
There was a problem hiding this comment.
Confirmation events are defined at this place in adrs.html, but many events were already predefined.
We can either define all events here, i.e. in addition please add above:
irsp_y
icb_y
ibor_icr
ibor_ipr
ibor_isd
ibor_non_icripd
ibor_icpd
ibor_iupd
ibor_ne
The second solution is to define events directly before creating parameters. Then the 4 definitions (please add icrsp_y_cr, icrsp_y_pr definitions as well) would have to be moved to the section "Derive Response Parameters requiring Confirmation".
There was a problem hiding this comment.
will keep before param
vignettes/irecist.Rmd
Outdated
| This article describes creating an `ADRS` ADaM with oncology endpoint parameters | ||
| based on iRECIST. It shows a similar way of deriving the endpoints presented in | ||
| [Creating ADRS (Including Non-standard Endpoints)](adrs.html). Most of the endpoints | ||
| are derived by calling `admiral::derive_extreme_event()`. Thus the examples in this |
There was a problem hiding this comment.
I agree , also not sure if at this point we want to bring IMWG in this vignette. I think in this section we should mention that we followed std. iRECIST guidelines as per the link below.
| and workflow could similarly be used to create an intermediary `ADEVENT` ADaM. | ||
|
|
||
| **Note**: *All examples assume CDISC SDTM and/or ADaM format as input | ||
| unless otherwise specified.* |
There was a problem hiding this comment.
@bundfussr @vinhn23 do you think we should mention that for SDTM RS domain test data for iRECIST responses we assumed all the iRECIST responses for target, non-target and overall are provided. If your study only collected iRECIST responses after RECIST1.1 progression..?
Also, can our function works when study only collects iRECIST responses after RECIST1.1 PD?
There was a problem hiding this comment.
add example for when Recist 1.1 and iRECIST data are collected together
| library(lubridate) | ||
| library(stringr) | ||
| data("adsl") | ||
| data("rs_onco_irecist") |
There was a problem hiding this comment.
can we add a comment for "rs_onco_irecist" #SDTM
| ) %>% | ||
| mutate(AVISIT = VISIT) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
In our data we don't have any partial date, so no imputation is applied. So, I think this is future sdtm update if we think is needed. just a thought, no action at this point
|
|
||
| Please note that the result `AVALC = "Y"` is defined by the first _two_ events | ||
| specified for `events`. For subjects with observations fulfilling both events | ||
| the one with the earlier date should be selected (and not the first one in the |
There was a problem hiding this comment.
This is an important point that the list in the event parameter consider all the objects and take the min. date and should be mentioned somewhere above in bold.
There was a problem hiding this comment.
keep as is. Text already in vignette
vignettes/irecist.Rmd
Outdated
| ovr = ovr, | ||
| adsl = adsl | ||
| ), | ||
| events = list(ibor_icr, ibor_ipr, ibor_isd, ibor_non_icripd, ibor_icpd, ibor_iupd, ibor_ne, no_data_missing), |
There was a problem hiding this comment.
@starosto I am not sure why the slides #66 mentioned PD as iBOR, we don't have PD as iRECIST response category. @vinhn23 @bundfussr For iBOR( unconfirmed) we need to think as this event list which use min. date value and after pick iUPD over iCPD.
Based on my discussion internally on this : we should only be assigning iUPD as BOR when a) no better response assessment exists, and b) when iCPD does not subsequently occur. When (any number of) iUPD TPRs are followed by iCPD, we should be considering them as a single block of confirmed PD for the purposes of deriving BOR and date of PD.
I checked the iBOR for 701-1028 , iBOR looks as I expected..
| ANL01FL = "Y" | ||
| ) | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
add ICBOR output as well. currently I don't see it.
There was a problem hiding this comment.
update to dataset_vignette
| - [Derive Response Parameter](#rsp) | ||
| - [Derive Clinical Benefit Parameter](#cb) | ||
| - [Derive Best Overall Response Parameter](#bor) | ||
| - [Derive Response Parameters requiring Confirmation](#confirm) |
There was a problem hiding this comment.
add ICBOR. so we have Yes and No best overall response parameters, we should have actual response parameters ( iBOR, iCBOR). we have CBR ( actual value) and ICPD. So , total 6 paramters.
| by_vars = exprs(STUDYID, USUBJID), | ||
| order = exprs(desc(AVALC), ADT), | ||
| mode = "first", | ||
| events = list(icrsp_y_icr, icrsp_y_ipr, icb_y, no_data_n), |
There was a problem hiding this comment.
Please update icb_y.
It has "condition = AVALC %in% c("iCR", "iPR", "iSD", "NON-iCR/NON-iUPD") & ADT >= RANDDT + 42" which include iCR and iPR.
In case when response is not confirmed we would assign Y because of icb_y. Consider 01-701-1239 as an example.
vignettes/irecist.Rmd
Outdated
| overwritten by `set_values_to` argument are kept from the earliest | ||
| occurring input record fulfilling the required criteria. | ||
|
|
||
| When an `iCPD` occurs,the date would be that of the first `iUPD`. |
There was a problem hiding this comment.
add a space in "occurs,the"
I would modify it a bit:
"When an iCPD occurs, the date of progression would be the same as RECIST 1.1 date (i.e. first iUPD date in that block/bar)."
vignettes/irecist.Rmd
Outdated
|
|
||
| icrsp_y_icr <- event_joined( | ||
| description = paste( | ||
| "Define confirmed response as iCR followed by iCR at least 28 days later and", |
There was a problem hiding this comment.
in description parameter also use confirmation_period instead of 28
vignettes/irecist.Rmd
Outdated
| ovr = ovr, | ||
| adsl = adsl | ||
| ), | ||
| events = list(ibor_icr, ibor_ipr, ibor_isd, ibor_non_icriupd, ibor_iupd, ibor_icpd, ibor_ne, no_data_missing), |
There was a problem hiding this comment.
ibor_icpd should be before ibor_iupd, so in case no better responses then iUPD and iCPD we would assing iCPD as iBOR
vignettes/irecist.Rmd
Outdated
| tmp_event_nr_var = event_nr, | ||
| order = exprs(event_nr,ADT), | ||
| mode = "first", | ||
| events = list(icbor_icr, icbor_ipr, ibor_isd, ibor_non_icriupd, ibor_iupd, ibor_icpd, ibor_ne, no_data_missing), |
There was a problem hiding this comment.
ibor_icpd should be before ibor_iupd, so in case no better responses then iUPD and iCPD we would assing iCPD as iBOR
vignettes/irecist.Rmd
Outdated
| ), | ||
| set_values_to = exprs( | ||
| PARAMCD = "ICBOR", | ||
| PARAM = "Immune Best Confirmed Overall Response by Investigator", |
There was a problem hiding this comment.
General note: I am not sure that the prefix "Immune" for PARAM variables is accurate. Maybe instead of the "Immune" prefix use "iRecist"?
vignettes/irecist.Rmd
Outdated
| ``` | ||
| ## Other Endpoints {#other} | ||
|
|
||
| For additional endpoints that can be added please see [Creating ADRS (Including Non-standard Endpoints)](adrs.html). |
There was a problem hiding this comment.
worth to mention which exactly parameters (not present here) are available there
…miralonco into 233_iRECIST_vignette_main
|
I have updated the iRECIST test data and added a section regarding mixing RECIST and iRECIST response values. The latest rendered version is available at https://bundfussr.github.io/admiral/233_iRECIST_vignette/articles/irecist.html. |
|
I am going to merge the iRECIST test data this evening because it is planned to submit the new pharmaversesdtm version to CRAN tomorrow. If you spot anything what should be changed, please comment on pharmaverse/pharmaversesdtm#41. |
| Another consideration could be extra potential protocol-specific sources of | ||
| Progressive Disease such as radiological assessments, which could be | ||
| pre-processed here to create a PD record to feed downstream derivations. |
There was a problem hiding this comment.
Feedback from Uwe:
I did not get what this means - I would delete
There was a problem hiding this comment.
This sentence also appears in Recist 1.1 vignette. I think that in some studies not only soft tissue lesions are assessed, but also for example bone lesions, and then we also have data from radiological evaluation. Based on both types of data, we proceed further.
There was a problem hiding this comment.
Will remove until example data is included.
|
Please add "Closes #" to the title of the pull request. Then the
issue is closed automatically once it is merged to
main.Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.
Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
develbranch until you have checked off each task.styler::style_file()to style R and Rmd filesdevtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelyNEWS.mdpkgdown::build_site()and check that all affected examples are displayed correctly and that all new functions occur on the "Reference" page.lintr::lint_package()R CMD checklocally and address all errors and warnings -devtools::check()The following line requests to update the man pages by the "Man Pages" workflow.
/roxygenize