-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Hi @baileych !
I went through and tested what I believe was all of the user facing functions in the argos package and am reporting back with some bugs I ran into, plus some other notes. Let me know if you have any questions or if it would be helpful to discuss!
results_tbl
Does not have functionality to support a NULL results_name_tag, so it may just need another ifelse to account for that. Using an empty “” achieves the same result though, so there is a current workaround.
qual_tbl
This is not currently being exported, which may be intentional but wanted to flag it since I know some people use qual_tbl on its own sometimes.
copy_to_new
When I tried this using a pipe (i.e. tbl %>% copy_to_new(df = .)) AND had an empty results_name_tag string, it errored because .parse_tblspec saw the . and assumed it was being used in a database table structure (i.e. dcc_pedsnet.person). dbExistsTable then couldn’t parse the empty string that got spit out as the table name, so it failed.
The function works fine in other scenarios (not using a pipe, using the pipe with a non-null results_name_tag), and this might be a user error as I’m not sure if it’s really built to function with dot logic.
compute_new
Would there potentially be a way to check the class of the data being fed into this and output a more informative error message if someone is trying to use this on a local / collected data frame? Just to make it more clear that copy_to_new is what should be used instead in that case (asking because I made this mistake while testing haha)
config_handle
This function did allow me to set a handler and access the configs like handler$config, but I’m not able to see my list of configs in the global environment. If I print the handler in the console, it prints something similar to what you get if you run get_argos_default() but with the added _co_req class list element. However, if I try to View(handler) or handler[[1]] I get the following error: Error in get0(label, envir = private$req_env) : invalid first argument. I can access a list of configs like below:
as.list.environment(handler[1]$.session$.__enclos_env__$private$req_env)
This also works for listing configs without the handler if you replace handler[1]$.session with get_argos_default().
lookback_facts
There was a typo in the params for this one. even_date_col should be event_date_col.
init_sum
The set_default parameter in init_sum has a defaulted argument in the primary list of parameters, but the default isn’t set in the nested list of parameters so it ends up being added as an extra column in the step log. This causes an issue with append_sum as well since the number of columns doesn’t align and breaks the union.
output_sum
The step_log and names parameters include references to self$ but I think that reference should only be included inside the function itself.