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

Bug: derive_param_computed() does nothing #2510

Open
yurovska opened this issue Sep 24, 2024 · 10 comments
Open

Bug: derive_param_computed() does nothing #2510

yurovska opened this issue Sep 24, 2024 · 10 comments
Assignees
Labels
bug Something isn't working programming

Comments

@yurovska
Copy link

yurovska commented Sep 24, 2024

What happened?

I'm using derive_param_computed() to derive a new parameter from other two. I also would like to keep a few variables from one of them. In the example below I take ADTM, ATMF, ADT, ADTF and ADY from "WSTCIR" parameter because it's visit-based whereas "HGHT" is measured only once at screening. I expect those variables to have the same values in the derived parameter. And it works fine unless any of those variables is equal to NA for all observations. In the current data there were no date imputations, so ADTF is NA everywhere. If I try to keep this variable in set_values_to then the function simply does nothing, with no errors, it just returns the input data frame with no new observations added at all. If I comment that line out then it works as expected.

advs_ratio <- advs %>%
  derive_param_computed(
    by_vars = exprs(USUBJID, VISIT),
    parameters = "WSTCIR",
    set_values_to = exprs(
      AVAL = AVAL.WSTCIR / AVAL.HGHT,
      ADTM = ADTM.WSTCIR,
      ATMF = ATMF.WSTCIR,
      ADT = ADT.WSTCIR,
      #ADTF = ADTF.WSTCIR,   # <--- If uncommented, WAISTHGT parameter is not derived at all
      ADY = ADY.WSTCIR,
      PARAMCD = "WAISTHGT",
      PARAM = "Waist-to-Height Ratio"
    ),
    constant_parameters = c("HGHT"),
    constant_by_vars = exprs(USUBJID)
  )

Session Information

No response

Reproducible Example

advs <- tribble(
  ~USUBJID,      ~PARAMCD, ~PARAM,        ~AVAL, ~AVALU, ~VISIT,      ~TESTVAR,
  "01-701-1015", "HEIGHT", "Height (cm)", 147.0, "cm",   "SCREENING", NA_character_,
  "01-701-1015", "WEIGHT", "Weight (kg)",  54.0, "kg",   "SCREENING", NA_character_,
  "01-701-1015", "WEIGHT", "Weight (kg)",  54.4, "kg",   "BASELINE",  NA_character_,
  "01-701-1015", "WEIGHT", "Weight (kg)",  53.1, "kg",   "WEEK 2",    NA_character_,
  "01-701-1028", "HEIGHT", "Height (cm)", 163.0, "cm",   "SCREENING", NA_character_,
  "01-701-1028", "WEIGHT", "Weight (kg)",  78.5, "kg",   "SCREENING", NA_character_,
  "01-701-1028", "WEIGHT", "Weight (kg)",  80.3, "kg",   "BASELINE",  NA_character_,
  "01-701-1028", "WEIGHT", "Weight (kg)",  80.7, "kg",   "WEEK 2",    NA_character_
)
  
advs_bmi <- advs %>%
  derive_param_computed(
    by_vars = exprs(USUBJID, VISIT),
    parameters = "WEIGHT",
    set_values_to = exprs(
      AVAL = AVAL.WEIGHT / (AVAL.HEIGHT / 100)^2,
      AVALU = "kg/m^2",
      TESTVAR = TESTVAR.WEIGHT,
      PARAMCD = "BMI",
      PARAM = "Body Mass Index (kg/m^2)"
    ),
    constant_parameters = c("HEIGHT"),
    constant_by_vars = exprs(USUBJID)
  )
@yurovska yurovska added bug Something isn't working programming labels Sep 24, 2024
@bundfussr
Copy link
Collaborator

Hi @yurovska , thanks for creating this issue.

Setting keep_nas = TRUE should solve the issue.

@yurovska
Copy link
Author

yurovska commented Sep 24, 2024

Hi @yurovska , thanks for creating this issue.

Setting keep_nas = TRUE should solve the issue.

But it will also add new observations even in those cases when the parameter could not be derived due to missing source parameters at certain visits or their blank AVAL values. I do not need that.

    ~USUBJID,      ~PARAMCD, ~PARAM,        ~AVAL, ~AVALU, ~VISIT,      ~TESTVAR,
    "01-701-1015", "HEIGHT", "Height (cm)", 147.0, "cm",   "SCREENING", NA_character_,
    "01-701-1015", "WEIGHT", "Weight (kg)",  54.0, "kg",   "SCREENING", NA_character_,
    "01-701-1015", "WEIGHT", "Weight (kg)",  54.4, "kg",   "BASELINE",  NA_character_,
    "01-701-1015", "WEIGHT", "Weight (kg)",  53.1, "kg",   "WEEK 2",    NA_character_,
    "01-701-1028", "HEIGHT", "Height (cm)", 163.0, "cm",   "SCREENING", NA_character_,
    "01-701-1028", "WEIGHT", "Weight (kg)",  78.5, "kg",   "SCREENING", NA_character_,
    "01-701-1028", "WEIGHT", "Weight (kg)",  80.3, "kg",   "BASELINE",  NA_character_,
    "01-701-1028", "WEIGHT", "Weight (kg)",  NA,   "kg",   "WEEK 2",    NA_character_
  )
  
  advs_bmi <- advs %>%
    derive_param_computed(
      by_vars = exprs(USUBJID, VISIT),
      parameters = "WEIGHT",
      set_values_to = exprs(
        AVAL = AVAL.WEIGHT / (AVAL.HEIGHT / 100)^2,
        AVALU = "kg/m^2",
        TESTVAR = TESTVAR.WEIGHT,
        PARAMCD = "BMI",
        PARAM = "Body Mass Index (kg/m^2)"
      ),
      constant_parameters = c("HEIGHT"),
      constant_by_vars = exprs(USUBJID),
      keep_nas = TRUE
    )

Screenshot 2024-09-24 at 14 53 49

@bundfussr
Copy link
Collaborator

That's correct. In this case you need to remove the undesired records afterwards, e.g., by adding %>% filter(!(PARAMCD == "BMI" & is.na(AVAL)).

@yurovska
Copy link
Author

Thanks @bundfussr. Of course I have already done it in my code but it's more like a workaround. I would still consider this an unexpected behaviour of the function.

@bundfussr
Copy link
Collaborator

Thanks @bundfussr. Of course I have already done it in my code but it's more like a workaround. I would still consider this an unexpected behaviour of the function.

I see. Maybe we should describe it clearer in the details section. We could also extend the keep_nas argument such that optionally a list of variables can be specified for which NA are acceptable. E.g., in the example above keep_nas = exprs(TESTVAR) could be specified to avoid the filter step.

@pharmaverse/admiral , what do you think?

@bms63
Copy link
Collaborator

bms63 commented Sep 24, 2024

I am happy for this update. Perhaps @yurovska wants to make the update? We will support you all the way @yurovska!

@yurovska
Copy link
Author

I am happy for this update. Perhaps @yurovska wants to make the update? We will support you all the way @yurovska!

That's fair. However, I'm afraid I'm too new to R, just started with it a month ago. But of course I will be more than happy to contribute at some point when I gain more experience. For the time being, bug reporting itself is my contribution.

@bms63
Copy link
Collaborator

bms63 commented Sep 24, 2024

I am happy for this update. Perhaps @yurovska wants to make the update? We will support you all the way @yurovska!

That's fair. However, I'm afraid I'm too new to R, just started with it a month ago. But of course I will be more than happy to contribute at some point when I gain more experience. For the time being, bug reporting itself is my contribution.

All good! Can we put you on as a reviewer once the update is completed?

@yurovska
Copy link
Author

I am happy for this update. Perhaps @yurovska wants to make the update? We will support you all the way @yurovska!

That's fair. However, I'm afraid I'm too new to R, just started with it a month ago. But of course I will be more than happy to contribute at some point when I gain more experience. For the time being, bug reporting itself is my contribution.

All good! Can we put you on as a reviewer once the update is completed?

Not sure what exactly it means and what I would be in change of as a reviewer, but why not :)

@bms63
Copy link
Collaborator

bms63 commented Sep 26, 2024

I am happy for this update. Perhaps @yurovska wants to make the update? We will support you all the way @yurovska!

That's fair. However, I'm afraid I'm too new to R, just started with it a month ago. But of course I will be more than happy to contribute at some point when I gain more experience. For the time being, bug reporting itself is my contribution.

All good! Can we put you on as a reviewer once the update is completed?

Not sure what exactly it means and what I would be in change of as a reviewer, but why not :)

Code Review/Pull Request Review is the coolest part of how we work in admiral. We need to discuss this update a little more internally at our next meeting, but once we arrive at the details someone will implement the change. You and another person will review that change and see if it meets the criteria laid out in the issue/request for change.

This is a great example, but not going to be as in-depth as this change. #2503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working programming
Development

No branches or pull requests

3 participants