pharmaverse / admiral

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

General Issue: Ensure argument name is correct in error messages #2404

Closed bundfussr closed 3 months ago

bundfussr commented 4 months ago

Background Information

For assert_*() calls where the argument is quoted (using enexpr()) the arg_name argument should be specified to display the correct argument name. For example in

my_expression = expr(VSTESTCD == "WEIGHT" & VISIT == "SCREENING")

derive_vars_merged(
  dm,
  dataset_add = select(vs, USUBJID, VSTESTCD, VISIT),
  by_vars = exprs(USUBJID),
  filter_add = my_expression
)
#> Error in `derive_vars_merged()`:
#> ! Argument `enexpr(filter_add)` must be a filter condition, but is a
#>   symbol

it should be "Argument filter_add must be a filter condition" instead of "Argument enexpr(filter_add) must be a filter condition".

Definition of Done

All error messages display the correct argument name.

bms63 commented 4 months ago

There might be more functions then just derive_vars_merged()

bms63 commented 4 months ago

Hi @ProfessorP-beep do you still have time to work on this? We have a release on June 3rd and want to have this updated.

ProfessorP-beep commented 4 months ago

Hey @bms63, Yea, was just reviewing again to start on it today since I have time to pick these back up this week.

ProfessorP-beep commented 4 months ago

@bms63 quick question, are the custom error messages written in each assert_()* function or kept in another script? I was going over the other related tickets as well.

bms63 commented 4 months ago

they are in the admiraldev repo

ProfessorP-beep commented 4 months ago

Do I need to be added as a contributor there as well? Just pulled it and working on the assert error.

Thanks.

bms63 commented 4 months ago

YEs please list yourself as a contributor. I hope you stay around!! If there is no activity for a while, we bump folks down to acknowledgements https://github.com/pharmaverse/admiral?tab=readme-ov-file#acknowledgments