Targets spec #76
Targets spec #76
Conversation
|
|
||
| # Main Export Functions | ||
|
|
||
| ### Verify Targets |
There was a problem hiding this comment.
Can you the document the list of warnings that this function can output
|
|
||
| The order in which modules are ran | ||
|
|
||
| # Example function usage |
There was a problem hiding this comment.
This section should be combined with the first section. Each example usage should be moved to the section for that function.
Targets_implementation.Rmd
Outdated
| #### Special Arguments | ||
|
|
||
| *role* This would search for variables matching the role in variables.csv and be replaced with vecor of var names during run time. | ||
| *data* This would pass the object attached to the bllflow object inside the data list ie: bllflow$data[[<whats inside data>]] |
There was a problem hiding this comment.
The data argument can also reference an object created during target execution. For example the second step in the depression imputation module is impute_depression_score. We can reference the result of this step in the next step using, data["impute_depression_scpre"]
There was a problem hiding this comment.
- It looks like you're missing the function that creates a targets list from modules map CSV or a module CSV file
- The column header for the CSV file should use the same naming convention as recodeflow (snake case)
- The spec should be updated to split the step_arguments column into the columns stepArgumentName and stepArgumentValue
- Can you update the spec with the description of the modules map file
- I think your tests are too complicated for testing what you want. Don't use the depression imputation module but create simpler module to specifically test what you want.
- Can you also add the flow diagram for the verify_targets function?
|
|
||
| *role* This would search for variables matching the role in variables.csv and be replaced with vecor of var names during run time. | ||
| *data* This would pass the object attached to the bllflow object inside the data list ie: bllflow$data[[<whats inside data>]] | ||
| *formula* This would create a left side = right side formula ie: formula[role["outcome"], role["predictor"], sep = "+"] would result in "outcome1 + outcome2 + outcome3 ~ predictor1 + predictor2 + predictor3" |
There was a problem hiding this comment.
How would include terms that are interations and those that are not using this notation?
There was a problem hiding this comment.
Do you mean interactions?
There was a problem hiding this comment.
Because in case of interactions a formula is not needed and you can pass variables as just a vector
There was a problem hiding this comment.
Can you give an example? Not sure I understand.
|
|
||
| The step_id column must contain a unique identifier for the step being performed. | ||
|
|
||
| ### Step_function |
There was a problem hiding this comment.
Lets make it so that if they do not provide a value for this column, we will just the value in the stepId column as the name of the function
There was a problem hiding this comment.
How would we handle multiple identical functions in the same module?
There was a problem hiding this comment.
I need step function to be the actual function name as thats what would be fed to targets.
There was a problem hiding this comment.
How would we handle multiple identical functions in the same module?
Since stepId has to be unique this won't be an issue for this feature
I need step function to be the actual function name as thats what would be fed to targets.
The function would have the same name as the value of the stepId if they do not put in a value for the step function column
| @@ -0,0 +1,7 @@ | |||
| Step_ID,Step_function,Step_arguments,Step_description,Step_order | |||
There was a problem hiding this comment.
Can you update these to be snakeCase like recode flow
|
|
||
| The step_arguments column would contain the arguments for the function including our shorthand (roles). I believe we should bring back the roles() and formula notations from previous implementation to avoid any confusion later. | ||
|
|
||
| #### Special Arguments |
There was a problem hiding this comment.
One addition would be a vector of values. For example an argument value for a list of survey cycles allowed, ["cchs2001_p", "cchs2003_p"]
There was a problem hiding this comment.
This would limit where a module can be used, I believe a column like this is best used in modules_map
There was a problem hiding this comment.
Not sure I understand. Can you give an example?
assets/specs/targets/modules_map.csv
Outdated
| @@ -0,0 +1,2 @@ | |||
| Module_Name,Module_Path,Module_Description,ModuleOrder | |||
There was a problem hiding this comment.
The headers should be converted to snakeCase
1A. Yeah sorry should this be a new function or a modification of any of the functions?
3A. Should we have an individual row per argument? |
1A. This should be a new function 3A. Yes, just like the original depression module I made here https://github.com/Big-Life-Lab/huiport/blob/master/worksheets/modules/depression-imputation.csv |
…nges in the PR by adding further description of modules_map
Spec and tests for the verification