Skip to content

Commit

Permalink
🐛 ✅ Make custom separator work when recursively flattening a dict par…
Browse files Browse the repository at this point in the history
…ameter even for nested dicts (#230)
  • Loading branch information
Galileo-Galilei committed Aug 20, 2021
1 parent af12c56 commit d626c8e
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
### Fixed

- :bug: Dictionnary parameters with integer keys are now properly logged in mlflow when ``flatten_dict_params`` is set to ``True`` in the ``mlflow.yml`` instead of raising a ``TypeError`` ([#224](https://github.com/Galileo-Galilei/kedro-mlflow/discussions/224))
- :bug: The user defined ``sep`` parameter of the ``mlflow.yml`` (defined in ``node`` section) is now used even if the parameters dictionnary has a depth>=2 ([#230](https://github.com/Galileo-Galilei/kedro-mlflow/issues/230))

### Changed

- :recycle: Move ``flatten_dict`` function to ``hooks.utils`` folder and rename it ``_flatten_dict`` to make more explicit that it is not a user facing function which should not be used directly.
- :recycle: Move ``flatten_dict`` function to ``hooks.utils`` folder and rename it ``_flatten_dict`` to make more explicit that it is not a user facing function which should not be used directly and comes with no guarantee. This is not considered as a breaking change since it is an undocumented function.


## [0.7.3] - 2021-08-16
Expand Down
6 changes: 5 additions & 1 deletion kedro_mlflow/framework/hooks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ def _assert_mlflow_enabled(pipeline_name: str) -> bool:
def _flatten_dict(d: Dict, recursive: bool = True, sep: str = ".") -> Dict:
def expand(key, value):
if isinstance(value, dict):
new_value = _flatten_dict(value) if recursive else value
new_value = (
_flatten_dict(value, recursive=recursive, sep=sep)
if recursive
else value
)
return [(f"{key}{sep}{k}", v) for k, v in new_value.items()]
else:
return [(f"{key}", value)]
Expand Down
12 changes: 12 additions & 0 deletions tests/framework/hooks/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,15 @@ def test_flatten_dict_with_float_keys():
6.7: 5, # 6.7 is not converted to string, but when the entire dict will be logged mlflow will take care of the conversion
},
}


def test_flatten_dict_with_used_defined_sep():
d = dict(a=1, b=dict(c=1, d=dict(e=3, f=dict(g=4, h=5))))

assert _flatten_dict(d=d, recursive=True, sep="_") == {
"a": 1,
"b_c": 1,
"b_d_e": 3,
"b_d_f_g": 4,
"b_d_f_h": 5,
}

0 comments on commit d626c8e

Please sign in to comment.