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

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

Closed
bundfussr opened this issue Apr 10, 2024 · 7 comments · Fixed by pharmaverse/admiraldev#450
Closed
Assignees

Comments

@bundfussr
Copy link
Collaborator

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
Copy link
Collaborator

bms63 commented Apr 17, 2024

There might be more functions then just derive_vars_merged()

@bms63
Copy link
Collaborator

bms63 commented May 8, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Collaborator

bms63 commented May 8, 2024

they are in the admiraldev repo

@ProfessorP-beep
Copy link
Collaborator

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

Thanks.

@bms63
Copy link
Collaborator

bms63 commented May 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment