search

RMI-PACTA / pacta.portfolio.allocate

https://rmi-pacta.github.io/pacta.portfolio.allocate/
Other
1 stars 0 forks source link

Deprecate `determine_tdm_variables` and define variables directly in `workflow.transition.monitor` #3

Closed jdhoffa closed 1 year ago

jdhoffa commented 1 year ago

We have a function in this package that defines configuration values for an upstream workflow repository. This seems backwards, since the purpose of the workflow.* repository is to configure and run the particular functions (with whatever arguments it wants).

I think it makes more sense to define these variables somewhere in workflow.transition.monitor, but still need to decide where/ how...

cjyetman commented 1 year ago

My understanding, which could be totally off base, was that this created a list of parameters to be passed to the main function that does the TDM calculation. Couldn't we even leave this function as is, but in the TM workflow not use it and rather specify the parameters explicitly?

jdhoffa commented 1 year ago

My understanding, which could be totally off base, was that this created a list of parameters to be passed to the main function that does the TDM calculation. Couldn't we even leave this function as is, but in the TM workflow not use it and rather specify the parameters explicitly?

Yup absolutely! But also might as well just remove the function if we don't need it.

I guess I was just wondering if you want to/ ar ok with specifying a bunch of arguments/ parameters directly in web_tool_script_2.R?

cjyetman commented 1 year ago

I guess I was just wondering if you want to/ ar ok with specifying a bunch of arguments/ parameters directly in web_tool_script_2.R?

seems legit to me