Remove custom Kedro syntax for --params (#3290)

* remove deprecation Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * simplify _split_params Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * remove _update_nested_dict, update params to use omegaconf, update magic_reload_kedro Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * split value Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * add validation back into _split_params * lint * lint Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * adjust tests * revert load_version test changes Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * update e2e test Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * changes based on review Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Update context.py Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Update __init__.py Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * changes based on review Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * add to release notes Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * Update RELEASE.md Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> * changes based on review Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> --------- Signed-off-by: SajidAlamQB <90610031+SajidAlamQB@users.noreply.github.com> Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com>
kedro-org · Nov 21, 2023 · 84b3e5c · 84b3e5c
1 parent 0fce6e3
commit 84b3e5c
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 90 deletions.
diff --git a/RELEASE.md b/RELEASE.md
@@ -53,6 +53,7 @@
 * The `spaceflights` starter has been renamed to `spaceflights-pandas`.
 
 ## Migration guide from Kedro 0.18.* to 0.19.*
+* Removed the custom Kedro syntax for `--params`, use the OmegaConf syntax instead by replacing `:` with `=`.
 
 ### DataSets
 * If you use `APIDataSet`, move all `requests` specific arguments (e.g. `params`, `headers`), except for `url` and `method`, to under `load_args`.

diff --git a/docs/source/configuration/parameters.md b/docs/source/configuration/parameters.md
@@ -112,11 +112,8 @@ The `kedro.framework.context.KedroContext` class uses the approach above to load
 
 Kedro also allows you to specify runtime parameters for the `kedro run` CLI command. Use the `--params` command line option and specify a comma-separated list of key-value pairs that will be added to [KedroContext](/kedro.framework.context.KedroContext) parameters and made available to pipeline nodes.
 
-Each key-value pair is split on the first colon or equals sign. The following examples are both valid commands:
+Each key-value pair is split on the first equals sign. The following example is a valid command:
 
-```bash
-kedro run --params=param_key1:value1,param_key2:2.0  # this will add {"param_key1": "value1", "param_key2": 2} to parameters dictionary
-```
 ```bash
 kedro run --params=param_key1=value1,param_key2=2.0
 ```
@@ -131,11 +128,8 @@ If any extra parameter key or value contains spaces, wrap the whole option conte
 kedro run --params="key1=value with spaces,key2=value"
 ```
 
-Since key-value pairs are split on the first colon or equals sign, values can contain colons/equals signs, but keys cannot. These are valid CLI commands:
+Since key-value pairs are split on the first equals sign, values can contain equals signs, but keys cannot. This is the valid CLI command:
 
-```bash
-kedro run --params=endpoint_url:https://endpoint.example.com
-```
 ```bash
 kedro run --params=endpoint_url=https://endpoint.example.com
 ```
diff --git a/docs/source/development/commands_reference.md b/docs/source/development/commands_reference.md
@@ -314,7 +314,7 @@ the names of relevant nodes, datasets, envs, etc. in your project.
 | `kedro run --config=<config_file_name>.yml`                         | Specify all command line options in a named YAML configuration file                                                                                                                                                                                     |
 | `kedro run --conf-source=<path_to_config_directory>`                | Specify a new source directory for configuration files                                                                                                                                                                                                  |
 | `kedro run --conf-source=<path_to_compressed file>`                 | Only possible when using the [``OmegaConfigLoader``](../configuration/configuration_basics.md#omegaconfigloader). Specify a compressed config file in `zip` or `tar` format.                                                                            |
-| `kedro run --params=<param_key1>:<value1>,<param_key2>:<value2>`    | Does a parametrised run with `{"param_key1": "value1", "param_key2": 2}`. These will take precedence over parameters defined in the `conf` directory. Additionally, dot (`.`) syntax can be used to address nested keys like `parent.child:value` |
+| `kedro run --params=<param_key1>=<value1>,<param_key2>=<value2>`    | Does a parametrised run with `{"param_key1": "value1", "param_key2": 2}`. These will take precedence over parameters defined in the `conf` directory. Additionally, dot (`.`) syntax can be used to address nested keys like `parent.child:value` |
 
 You can also combine these options together, so the following command runs all the nodes from `split` to `predict` and `report`:
 

diff --git a/features/run.feature b/features/run.feature
@@ -57,6 +57,6 @@ Feature: Run Project
   Scenario: Run kedro run with extra parameters
     Given I have prepared a config file
     And I have run a non-interactive kedro new with starter "default"
-    When I execute the kedro command "run --params extra1:1,extra2:value2"
+    When I execute the kedro command "run --params extra1=1,extra2=value2"
     Then I should get a successful exit code
     And the logs should show that 4 nodes were run
diff --git a/kedro/framework/cli/utils.py b/kedro/framework/cli/utils.py
@@ -432,27 +432,24 @@ def _split_params(ctx, param, value):
     dot_list = []
     for item in split_string(ctx, param, value):
         equals_idx = item.find("=")
-        colon_idx = item.find(":")
-        if equals_idx != -1 and colon_idx != -1 and equals_idx < colon_idx:
-            # For cases where key-value pair is separated by = and the value contains a colon
-            # which should not be replaced by =
-            pass
-        else:
-            item = item.replace(":", "=", 1)  # noqa: PLW2901
-        items = item.split("=", 1)
-        if len(items) != 2:  # noqa: PLR2004
+        if equals_idx == -1:
+            # If an equals sign is not found, fail with an error message.
             ctx.fail(
                 f"Invalid format of `{param.name}` option: "
-                f"Item `{items[0]}` must contain "
-                f"a key and a value separated by `:` or `=`."
+                f"Item `{item}` must contain a key and a value separated by `=`."
             )
-        key = items[0].strip()
+        # Split the item into key and value
+        key, _, val = item.partition("=")
+        key = key.strip()
         if not key:
+            # If the key is empty after stripping whitespace, fail with an error message.
             ctx.fail(
                 f"Invalid format of `{param.name}` option: Parameter key "
                 f"cannot be an empty string."
             )
-        dot_list.append(item)
+        # Add "key=value" pair to dot_list.
+        dot_list.append(f"{key}={val}")
+
     conf = OmegaConf.from_dotlist(dot_list)
     return OmegaConf.to_container(conf)
 

diff --git a/kedro/framework/context/context.py b/kedro/framework/context/context.py
@@ -9,6 +9,7 @@
 from warnings import warn
 
 from attrs import field, frozen
+from omegaconf import OmegaConf
 from pluggy import PluginManager
 
 from kedro.config import AbstractConfigLoader, MissingConfigException
@@ -137,23 +138,6 @@ def _validate_transcoded_datasets(catalog: DataCatalog):
         _transcode_split(dataset_name)
 
 
-def _update_nested_dict(old_dict: dict[Any, Any], new_dict: dict[Any, Any]) -> None:
-    """Update a nested dict with values of new_dict.
-
-    Args:
-        old_dict: dict to be updated
-        new_dict: dict to use for updating old_dict
-
-    """
-    for key, value in new_dict.items():
-        if key not in old_dict:
-            old_dict[key] = value
-        elif isinstance(old_dict[key], dict) and isinstance(value, dict):
-            _update_nested_dict(old_dict[key], value)
-        else:
-            old_dict[key] = value
-
-
 def _expand_full_path(project_path: str | Path) -> Path:
     return Path(project_path).expanduser().resolve()
 
@@ -196,8 +180,12 @@ def params(self) -> dict[str, Any]:
         except MissingConfigException as exc:
             warn(f"Parameters not found in your Kedro project config.\n{str(exc)}")
             params = {}
-        _update_nested_dict(params, self._extra_params or {})
-        return params
+
+        if self._extra_params:
+            # Merge nested structures
+            params = OmegaConf.merge(params, self._extra_params)
+
+        return OmegaConf.to_container(params) if OmegaConf.is_config(params) else params
 
     def _get_catalog(
         self,

diff --git a/tests/framework/cli/test_cli.py b/tests/framework/cli/test_cli.py
@@ -682,7 +682,7 @@ def test_run_with_invalid_config(
         [
             ({}, {}),
             ({"params": {"foo": "baz"}}, {"foo": "baz"}),
-            ({"params": "foo:baz"}, {"foo": "baz"}),
+            ({"params": "foo=baz"}, {"foo": "baz"}),
             (
                 {"params": {"foo": "123.45", "baz": "678", "bar": 9}},
                 {"foo": "123.45", "baz": "678", "bar": 9},
@@ -727,10 +727,9 @@ def test_run_with_params_in_config(
     @mark.parametrize(
         "cli_arg,expected_extra_params",
         [
-            ("foo:bar", {"foo": "bar"}),
             ("foo=bar", {"foo": "bar"}),
             (
-                "foo:123.45, bar:1a,baz:678. ,qux:1e-2,quux:0,quuz:",
+                "foo=123.45, bar=1a,baz=678. ,qux=1e-2,quux=0,quuz=",
                 {
                     "foo": 123.45,
                     "bar": "1a",
@@ -740,19 +739,18 @@ def test_run_with_params_in_config(
                     "quuz": None,
                 },
             ),
-            ("foo:bar,baz:fizz:buzz", {"foo": "bar", "baz": "fizz:buzz"}),
-            ("foo=fizz:buzz", {"foo": "fizz:buzz"}),
-            ("foo:fizz=buzz", {"foo": "fizz=buzz"}),
+            ("foo=bar,baz=fizz=buzz", {"foo": "bar", "baz": "fizz=buzz"}),
+            ("foo=fizz=buzz", {"foo": "fizz=buzz"}),
             (
-                "foo:bar, baz: https://example.com",
+                "foo=bar, baz= https://example.com",
                 {"foo": "bar", "baz": "https://example.com"},
             ),
-            ("foo:bar, foo:fizz buzz", {"foo": "fizz buzz"}),
-            ("foo:bar,baz:fizz buzz", {"foo": "bar", "baz": "fizz buzz"}),
-            ("foo.nested:bar", {"foo": {"nested": "bar"}}),
+            ("foo=bar, foo=fizz buzz", {"foo": "fizz buzz"}),
+            ("foo=bar,baz=fizz buzz", {"foo": "bar", "baz": "fizz buzz"}),
+            ("foo.nested=bar", {"foo": {"nested": "bar"}}),
             ("foo.nested=123.45", {"foo": {"nested": 123.45}}),
             (
-                "foo.nested_1.double_nest:123.45,foo.nested_2:1a",
+                "foo.nested_1.double_nest=123.45,foo.nested_2=1a",
                 {"foo": {"nested_1": {"double_nest": 123.45}, "nested_2": "1a"}},
             ),
         ],
@@ -776,18 +774,18 @@ def test_run_extra_params(
             env=mocker.ANY, conf_source=None, extra_params=expected_extra_params
         )
 
-    @mark.parametrize("bad_arg", ["bad", "foo:bar,bad"])
+    @mark.parametrize("bad_arg", ["bad", "foo=bar,bad"])
     def test_bad_extra_params(self, fake_project_cli, fake_metadata, bad_arg):
         result = CliRunner().invoke(
             fake_project_cli, ["run", "--params", bad_arg], obj=fake_metadata
         )
         assert result.exit_code
         assert (
-            "Item `bad` must contain a key and a value separated by `:` or `=`."
+            "Item `bad` must contain a key and a value separated by `=`."
             in result.stdout
         )
 
-    @mark.parametrize("bad_arg", [":", ":value", " :value"])
+    @mark.parametrize("bad_arg", ["=", "=value", " =value"])
     def test_bad_params_key(self, fake_project_cli, fake_metadata, bad_arg):
         result = CliRunner().invoke(
             fake_project_cli, ["run", "--params", bad_arg], obj=fake_metadata

diff --git a/tests/framework/context/test_context.py b/tests/framework/context/test_context.py
@@ -21,7 +21,6 @@
 from kedro.framework.context.context import (
     _convert_paths_to_absolute_posix,
     _is_relative_path,
-    _update_nested_dict,
 )
 from kedro.framework.hooks import _create_hook_manager
 from kedro.framework.project import (
@@ -423,35 +422,3 @@ def test_convert_paths_to_absolute_posix_converts_full_windows_path_to_posix(
     project_path: Path, input_conf: dict[str, Any], expected: dict[str, Any]
 ):
     assert _convert_paths_to_absolute_posix(project_path, input_conf) == expected
-
-
-@pytest.mark.parametrize(
-    "old_dict, new_dict, expected",
-    [
-        (
-            {
-                "a": 1,
-                "b": 2,
-                "c": {
-                    "d": 3,
-                },
-            },
-            {"c": {"d": 5, "e": 4}},
-            {
-                "a": 1,
-                "b": 2,
-                "c": {"d": 5, "e": 4},
-            },
-        ),
-        ({"a": 1}, {"b": 2}, {"a": 1, "b": 2}),
-        ({"a": 1, "b": 2}, {"b": 3}, {"a": 1, "b": 3}),
-        (
-            {"a": {"a.a": 1, "a.b": 2, "a.c": {"a.c.a": 3}}},
-            {"a": {"a.c": {"a.c.b": 4}}},
-            {"a": {"a.a": 1, "a.b": 2, "a.c": {"a.c.a": 3, "a.c.b": 4}}},
-        ),
-    ],
-)
-def test_update_nested_dict(old_dict: dict, new_dict: dict, expected: dict):
-    _update_nested_dict(old_dict, new_dict)  # _update_nested_dict change dict in place
-    assert old_dict == expected
diff --git a/tests/ipython/test_ipython.py b/tests/ipython/test_ipython.py
@@ -226,7 +226,7 @@ def test_load_extension_register_line_magic(self, mocker, ipython):
             ". --env=base",
             "--env=base",
             "-e base",
-            ". --env=base --params=key:val",
+            ". --env=base --params=key=val",
             "--conf-source=new_conf",
         ],
     )