fix(execute.py): fixing bad arguments for cli functions

[odarotto]

Instead of calling the func.main() with the reformated arguments, we call the func.callback() that accepts the Python formatted arguments.

[chrisandrade716]

the issue with this is it will ignore all of Click's middleware that many people rely on for their science functions. for example, many people don't pass in all parmaeters because there is a default set with Click. it will also ignore checking of valid parameters (e.g. with click.Choice), and probably most importantly it will ignore auto-formatting done with e.g. type=click.DateTime. there are ways around this, but such a change would require people to modify their functions (at a rather unoppurtune time as i am leaving :/).

i think a Click command should be called as such, and not a pythonic function anymore. it's risky

[odarotto]

the issue with this is it will ignore all of Click's middleware that many people rely on for their science functions. for example, many people don't pass in all parmaeters because there is a default set with Click. it will also ignore checking of valid parameters (e.g. with click.Choice), and probably most importantly it will ignore auto-formatting done with e.g. type=click.DateTime. there are ways around this, but such a change would require people to modify their functions (at a rather unoppurtune time as i am leaving :/).

One option is to ask them to have a module with the CLI version of the functions (e.g. click functions) and other module with the real functions, the other case is to implement good programming practices in the code (define defaults for required arguments, implement checking in the function itself) but in any case they'll need to make changes in their code.

i think a Click command should be called as such, and not a pythonic function anymore. it's risky

Yes, these kind of commands should not be called as Python functions but if we insist in running these functions we cannot treat them as commands in a context that is designed to handle functions, hence the changes in this PR

[chrisandrade716]

i 100% agree we should have better coding practices, but i'm leaving CHIME tomorrow and i won't be able to help the SPS team do that. who will enforce or guide that? it would be tough on them to merge this change just as i leave. we have some type checking in the function, defaults in the function definition, and specify most parameters directly. but it's not everywhere, because once you make something a Click command, it is no longer a regular Python function. thus, this change is breaking to anyone using Click commands. is there no one else, e.g. on FRB, who use Workflow whose functions are Click commands? there could be a lot of unexpected behaviour, so i don't think we should treat this merge casually with just us two reviewing only.

i agree that it was a strange decision to remove work.command functionality and merge that into work.function. and yes it make sense that work.function should not be trying to do command things.

1. will this be announced somewhere visibile to SPS team members so they can understand the ramifications?
2. why not make a separate issue for such a high-level impactful change?
3. it is of my understanding this was brought up because a list parameter was not being parsed correctly. why don't we address that issue specifically first, before planning out the deployment of this larger change?

i'm not trying to push back on a good idea you've implemented, but this is removing a large piece of code, and i just want to make sure it's not going under the radar without additional review and proper notice.

[chrisandrade716]

one idea if we really want to go this route right this instance: let's not leave the Click Command function hanging without any usage, and perhaps revive work.command functionality here? otherwise we break things with no recovery option. again, we have to make an annoucement to various teams using Click commands.