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

Add target_variable and target_outcome to task ids as potential task ids #29

Closed
annakrystalli opened this issue Jan 5, 2023 · 3 comments · Fixed by #59
Closed

Add target_variable and target_outcome to task ids as potential task ids #29

annakrystalli opened this issue Jan 5, 2023 · 3 comments · Fixed by #59
Milestone

Comments

@annakrystalli
Copy link
Contributor

This reflects the fact we include target as a potential task_id and target_variable and target_outcome are an option that has been discussed.

One question I had is that while using a single target task id, optional and required arrays of multiple unique targets are explicit, what happens when you have multiple targets but spread across target_variable and target_outcome. How do you ensure they specify unique and correct combinations? Does the order they are specified in the array become important?

e.g. if two valid targets where inc hosp & inc case, how would they be specified?

"target_variable": {
  "required": null,
  "optional": ["hosp", "case"]
},
"target_outcome": {
  "required": null,
  "optional": ["inc", "inc"]
},

@elray1 your thoughts would be appreciated!

@annakrystalli annakrystalli modified the milestones: v1.0.1, v2.0.0 Jul 7, 2023
@annakrystalli
Copy link
Contributor Author

@elray1 @nickreich , should I go ahead and add these two standard tasks IDs (as part of a two column target hub) to version 2.0.0?

@annakrystalli
Copy link
Contributor Author

For completeness, the answer to the above question is:

"target_variable": {
  "required": null,
  "optional": ["hosp", "case"]
},
"target_outcome": {
  "required":["inc"],
  "optional": null
},

@annakrystalli annakrystalli mentioned this issue Jul 12, 2023
@elray1
Copy link
Contributor

elray1 commented Jul 12, 2023

I don't have a strong opinion about whether or not we include this in v2.0.0. on one hand, kind of seems like we might as well? on the other hand, this is not a high priority and i think adding it in later would not be a breaking change and so would not require a new major version if we put it off?

@annakrystalli annakrystalli linked a pull request Jul 13, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants