Targets spec #76

rvyuha · 2021-05-04T18:14:18Z

Spec and tests for the verification

yulric · 2021-05-17T19:24:44Z

Targets_implementation.Rmd

+
+# Main Export Functions
+
+### Verify Targets


Can you the document the list of warnings that this function can output

yulric · 2021-05-17T19:25:56Z

Targets_implementation.Rmd

+
+The order in which modules are ran
+
+# Example function usage


This section should be combined with the first section. Each example usage should be moved to the section for that function.

yulric · 2021-05-17T19:37:22Z

Targets_implementation.Rmd

+#### 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>]]


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"]

yulric

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?

yulric · 2021-05-17T19:40:12Z

Targets_implementation.Rmd

+
+*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"


How would include terms that are interations and those that are not using this notation?

Do you mean interactions?

Because in case of interactions a formula is not needed and you can pass variables as just a vector

Can you give an example? Not sure I understand.

yulric · 2021-05-17T19:43:28Z

Targets_implementation.Rmd

+
+The step_id column must contain a unique identifier for the step being performed.
+
+### Step_function


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

How would we handle multiple identical functions in the same module?

I need step function to be the actual function name as thats what would be fed to targets.

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

yulric · 2021-05-17T19:44:04Z

assets/specs/targets/depression_imputation_module.csv

@@ -0,0 +1,7 @@
+Step_ID,Step_function,Step_arguments,Step_description,Step_order


Can you update these to be snakeCase like recode flow

yulric · 2021-05-17T19:46:08Z

Targets_implementation.Rmd

+
+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


One addition would be a vector of values. For example an argument value for a list of survey cycles allowed, ["cchs2001_p", "cchs2003_p"]

This would limit where a module can be used, I believe a column like this is best used in modules_map

Not sure I understand. Can you give an example?

yulric · 2021-05-17T19:47:08Z

assets/specs/targets/modules_map.csv

@@ -0,0 +1,2 @@
+Module_Name,Module_Path,Module_Description,ModuleOrder


The headers should be converted to snakeCase

rvyuha · 2021-05-25T17:30:53Z

It looks like you're missing the function that creates a targets list from modules map CSV or a module CSV fill

1A. Yeah sorry should this be a new function or a modification of any of the functions?

The spec should be updated to split the step_arguments column into the columns stepArgumentName and
stepArgumentValue

3A. Should we have an individual row per argument?

yulric · 2021-05-26T11:41:27Z

It looks like you're missing the function that creates a targets list from modules map CSV or a module CSV fill

1A. Yeah sorry should this be a new function or a modification of any of the functions?

The spec should be updated to split the step_arguments column into the columns stepArgumentName and
stepArgumentValue

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

rvyuha added 3 commits April 20, 2021 13:52

[Feature] Added targets spec

7601eda

.

dbcae49

[Feature] Added module verification tests

8a25d39

rvyuha requested a review from yulric May 4, 2021 18:14

DougManuel self-requested a review May 17, 2021 19:21

yulric reviewed May 17, 2021

View reviewed changes

yulric requested changes May 17, 2021

View reviewed changes

[Feature] Added flow diagram for verification and addressed minor cha…

82156d5

…nges in the PR by adding further description of modules_map

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Targets spec #76

Targets spec #76

rvyuha commented May 4, 2021

yulric May 17, 2021

yulric May 17, 2021

yulric May 17, 2021

yulric left a comment •

edited

Loading

yulric May 17, 2021

rvyuha May 25, 2021

rvyuha May 25, 2021

yulric May 26, 2021

yulric May 17, 2021

rvyuha May 25, 2021

rvyuha May 25, 2021

yulric May 26, 2021

yulric May 17, 2021

yulric May 17, 2021

rvyuha May 25, 2021

yulric May 26, 2021

yulric May 17, 2021

rvyuha commented May 25, 2021

yulric commented May 26, 2021


		The step_id column must contain a unique identifier for the step being performed.

		### Step_function

		@@ -0,0 +1,7 @@
		Step_ID,Step_function,Step_arguments,Step_description,Step_order


		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

		@@ -0,0 +1,2 @@
		Module_Name,Module_Path,Module_Description,ModuleOrder

Targets spec #76

Are you sure you want to change the base?

Targets spec #76

Conversation

rvyuha commented May 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yulric left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvyuha commented May 25, 2021

yulric commented May 26, 2021

yulric left a comment •

edited

Loading