Updated narg parsing. Documentation. Testing.

[cigani]

• Modified nargs in the following ways:

1. nargs not explicitly provided are not passed to the generated kwargs dictionary
2. nargs can optionally have their type specified
3. types of nargs are checked against a simple lookup dictionary
4. tried to increase the feedback (error messages) where behavior might not be immediately transparent to the user

• Updated the documentation to be a bit more clear about how default arguments are parsed
• Added a few more tests to ensure functionality of default and type specified arguments

[nat-n]

Hey thanks again for working on this. It's good to see progress!

I guess by nargs you mean "named args" rather than the argparse nargs option?

I've left a first round of comments. I still need to check it out and play around with it, maybe think through a few more scenarios for script args, and properly review the tests.

[cigani]

Thanks for the feedback nat. I'll try to realign the PR. Hopefully I'll get you something updated this weekend since I have to work this weekend anyway :D

[nat-n]

I think we shouldn't require the use of kwargs (script.py:48), since it creates a coupling between the task definition and the function definition, whereby even if you define the call params using positional arguments, you actually have to make the variables names line up with the names from the function def anyway or it won't work. Also if you want to call a function that accepts *args you're screwed...

For context, a use-case I'd like to encourage is using poe to create a quick and convenient CLI for a python function from any random library with just a couple lines of config! But this means the user might not control the function signature they're interfacing with.

---

Also from seeing the default_value example in your test case and playing around I realise we have a bigger problem with how we handle call_params and need to rethink a bit. The way it works now (since my original implementation for named args) will cause a breaking change and loss of functionality vs 0.10.0.

That default_value in script = "pkg:fun(foo=default_value)" gets interpreted as a string is weird!

For consistency the following should work as one would naively expect:

[tool.poe.tasks.call]
script = "pkg:fun(foo='foo', bar=1, baz=1.0, qux=true, vroom=CLI_ARG, shell_path=environ.get('PATH'))"
args = ["CLI_ARG"]

Note this is assuming we put add from os import environ to the setup code which I think makes sense to enable this kind of usage, though that's probably a feature for after 0.11.0.

I think we should start with a failing test case like that one, and coordinate a bit so we can both push some changes to this branch without stepping on each others toes.

Thinking on solutions; One alternative approach would be to inject all the named argument values as locals in the scope where the function is called. Then maybe we don't need to edit the call_params string at all! Instead we just expect it to be valid python (maybe with a couple of checks against code injection shenanigans). The downside of this is that we need to think about how to deal with collisions. Eg. if a user wants a cli arg called locals. etc

Another option would be to inject the named args like is done for cmd tasks, e.g.

[tool.poe.tasks.call]
script = "pkg:fun(foo='foo', bar=1, baz=1.0, qux=true, vroom=${CLI_ARGS}, shell_path=environ.get('PATH'))"
args = ["CLI_ARG"]

Though this seems kind of weird...

Anything else probably requires some level of parsing python syntax to modify the call_args string with confidence; at least to handle primitive types, (which means quote matching etc, not just naively splitting on , as a first step 🙀 ). Maybe there is an easy way to achieve this... I think it's my preferred solution but more research is required.

What do you think?

Of course we can break this problem up, and leave parts of it to solve in follow up PRs/releases, as long as we don't box ourselves in.

[cigani]

[tool.poe.tasks.call]
script = "pkg:fun(foo='foo', bar=1, baz=1.0, qux=true, vroom=${CLI_ARGS}, shell_path=environ.get('PATH'))"
args = ["CLI_ARG"]

As far as I can tell this doesn't work natively. All those "Default Values" will be interpreted as strings. This would be a good base for a failing test but I think to make it function properly we'd need to exploit TOML types a bit more efficiently

script = "pkg:fun", { foo='foo', bar=1, baz=1.0, qux=true, vroom=${CLI_ARGS}, shell_path=environ.get('PATH')) }
args = ["CLI_ARG"]

Basically taking the arguments outside of the " " so the TOML can make sense of the type when generating the underling dictionary utilized. An example of this working well is

  [tool.poe.tasks.greet-rekeyed.args.value]
    default=false

Here false will be translated by tomli to False as expected.

[nat-n]

not all of these tests are focused on cmd task logic.