kedro run --params key=1,2,3 Kedro CLI does not take list as argument

[noklam]

Description

Short description of the problem here.
Some kedro cli like kedro run --tags accept argument with comma-separated list. This is not true with the --params as , get parsed as a separate parameters.

kedro run --params error:1,2 or kedro run --params error:[1,2] won't work`

You will get some error like this.

Error: Invalid format of `params` option: Item `b` must contain a key and a value separated by `:` or `=`.
(kedro_core) soft-runner main % kedro run --params error:a,b

The reason that I need this is I want to create my hooks, which takes CLI arguments and execute custom logic. The --params is the most intuitive argument I can find, and I can get it from before_pipeline_run's run_params.

The alternative is to add my custom CLI flag. This has 2 disadvantage.

1. I need the cli.py
2. I cannot get it from the hooks spec because run_params is hard-coded
   This creates a tight couple between my cli file and my hooks. Using params does not requires any of this, and the CLI already handle all the parsing (with OmegaConf) for me.

Context

How has this bug affected you? What were you trying to accomplish?
For some reason, I remember this works in older versions (??) or maybe the only way is to use the --config flag but that flag wasn't well used either.
I want my hooks reading the CLI, one of the easiest way is to use the --params. Even if I want to parse , myself, the CLI will not allow it. My alternative is to use other delimeter like &

Steps to Reproduce

kedro run --params error:a=1,2 should return a dictionary like {"a": [1,2]}

Expected Result

Tell us what should happen.
Maybe it never worked before, or the documentation is missing.

Actual Result

Tell us what happens instead.
I get an error

-- If you received an error, place it here.

-- Separate them if you have more than one.

Your Environment

Include as many relevant details about the environment in which you experienced the bug:

• Kedro version used (pip show kedro or kedro -V): 0.18.7
• Python version used (python -V): 3.9.1
• Operating system and version:

[antonymilne]

Just a few quick comments, without trying it out myself...

• in future we should delegate parsing of --params as much as possible to OmegaConf.from_dotlist. The : syntax should be considered deprecated. See Remove custom Kedro syntax for --params #1965
• hence this should work: kedro run --params a=[1, 2, 3]. Since you can do OmegaConf.from_dotlist(["a=[1, 2, 3]"]). If this doesn't already work then it's a (not very important) problem with our implementation of _split_params @ankatiyar
• with all that said, there is a remaining problem with how you would set multiple parameters using the dotlist syntax which we have not discussed before. i.e. how to do kedro run --params a=1,b=2. Note that OmegaConf.from_dotlist requires a list of strings (i.e. ["a=1", "b=2"] rather than "a=1,b=2") so we'll still need to do some splitting or something here
• @AhdraMeraliQB spent a while thinking about what the right behaviour for these different singular/plural CLI flags should be a how to specify multiple options, but I don't think the above was considered (I only just thought of it). We should figure this out...

Just to clarify: even though we should delegate to OmegaConf.from_dotlist for this, it shouldn't be tried to the config loader in any way. We're just using their syntax for parsing params from the CLI.

[marrrcin]

@antonymilne what if indeed this (parsing the --params) was offloaded to the config loader class 🤔? That would actually make this really customizable with sensible default.

FYI, in our plugins we're actually using JSON format to pass the params, e.g. https://github.com/getindata/kedro-azureml/blob/5ae7bcf430ec0a4824b6a198c4e919ae444e2589/kedro_azureml/cli.py#L236

[noklam]

That would actually make this really customizable with sensible default.

What do you mean? The proposal does not requires using ConfigLoader, it only use the OmegaConf cli parse feature. What extra feature is enabled to use config loader?

[marrrcin]

What extra feature is enabled to use config loader?

Someone could e.g. implement custom parsing of CLI args - like the JSON example.

[noklam]

A quick attempt as I come across some question in Slack today.

Originally I want to take regex approach https://stackoverflow.com/questions/26633452/how-to-split-by-commas-that-are-not-within-parentheses but without success.

#3327 - this is not ready to be merged. This only attempt to support [] but not {}, I found it quite difficult to parse string correctly so I limit the scope for brackets only. On the other hand, list is more CLI friendly where typing out the entire dictionary in CLI is overcomplicated.

The key is try to parse the comma correctly, that is, ignoring comma inside [] () {}, the remaining part will basically just work because OmegaConf.from_dotlist handles list and dict by default.

[marrrcin]

What's the status of this? 🤔

[noklam]

It was never put into a sprint, I made a draft PR to see what it takes to add just list as an example.

It requires building a parser basically, though in this case maybe we can just add list and forget about dict and everything.

[astrojuanlu]

Is there some prior art of CLI tools accepting Python primitive non-scalar types on the CLI?

[datajoely]

Some overlap with this recent discussion #3904 (comment)