Omegaconf breaks with a custom resolver providing a datetime

[ankatiyar]

Description

This case came up in a user question (@PetitLepton) - https://linen-slack.kedro.org/t/16417334/hi-folks-i-encountered-an-unexpected-behavior-regarding-the-#8d5f69d7-25ce-409f-b009-0a6a50cd064c

Context

The user has a custom resolver in their config which converts a string to a pandas.Timestamp type -

• settings.py :

from datetime import timezone

# Third-party
import pandas


def parse_timezone_aware_datetime_string(datetime_string: str) -> pandas.Timestamp:
    timestamp = pandas.Timestamp(datetime_string).astimezone(tz=timezone.utc)
    return timestamp


# Keyword arguments to pass to the `CONFIG_LOADER_CLASS` constructor.
CONFIG_LOADER_ARGS = {
    "custom_resolvers": {
        "parse_timezone_aware_datetime_string": parse_timezone_aware_datetime_string,
    },
    "base_env": "base",
    "default_run_env": "local",
     }
}

• parameters.yml

start: ${parse_timezone_aware_datetime_string:2023-12-13T23:00:00+01:00}
test: "${runtime_params:the_test,2}"

This works fine when you do a kedro run without any run time parameters but fails with the following error when you do a kedro run --params="the_test=1":

omegaconf.errors.UnsupportedValueType: Value 'datetime' is not a supported primitive type
    full_key: start
    object_type=dict

This is because when there's run time params, there is an additional merge step -

kedro/kedro/framework/context/context.py

Lines 203 to 205 in 80ad182

	if self._extra_params:
	# Merge nested structures
	params = OmegaConf.merge(params, self._extra_params)

This happens after the parameter with the custom resolver has already been resolved into the pandas.Timestamp type and omegaconf apparently does not support non primitive types in the config.

Possible Implementation

• Delay the resolution of the config to after this merge happens in the Context

Possible Alternatives

• Use omegaconf's DictConfig type instead of converting to dict as we do right now, where the resolution happens at access time. Related issue: Spike: Enable access to OmegaConfigLoader DictConfig instead of dict only #2973
• Also potentially related - [Proposal] - Replace ConfigLoader with ConfigLoader and ConfigResolver #2481

[noklam]

Should this get moved to discussion or is it ready to be discussed? @ankatiyar

[astrojuanlu]

This hasn't been discussed yet, it's in the Inbox

[noklam]

84b3e5c

This was the original PR that creates this regression. The PR was about removing custom syntax, there is a minor refactoring that use OmegaConf to remove the custom nest dict update function. This is the root cause of the bug.

Things to do IMO:

• Revert back to custom nested dict update
• Added a regression use case with non-primitive type.

Another way to confirm this is to reproduce the error and run this in older version (maybe 0.18.14)

[PetitLepton]

I am coming back to this as I currently cannot bump our code from 0.18.14 to 0.19.8. A full minimal example is at https://github.com/PetitLepton/kedro-runtime-params.

[noklam]

@PetitLepton thanks for bumping this, I think we have a clear solution here already, which is do not use Omegaconf in KedroContext. I have commented that we should not do this , the simple solution here is to revert to a recursively dict update.