Default for missing nested config should be the default state of the nested config

[stephen-bunn]

Expected Behavior

Given the following:

from textwrap import dedent
from file_config import config, var, build_schema, utils, validate, to_dict

@config
class TestConfig:
    @config
    class InnerConfig:
        foo = var(str, default="Default", required=False)

    inner = var(InnerConfig, required=False)
    bar = var(bool, default="Default", required=False)


json_ = dedent(
    """\
    {
        "bar": false
    }
"""
)
c = TestConfig.loads_json(json_)
print(c)

I would expect the value of the parsed config instance would be TestConfig(inner=TestConfig.InnerConfig(foo="Default"), bar=False).

Current Behavior

Given the above example, the loaded TestConfig instance is TestConfig(inner=None, bar=False).

Possible Solution

The solution that would solve this case is by changing the handler for building config types in _file_config._build...

elif is_config_type(entry.type):
    if arg_key not in dictionary:
-        kwargs[var.name] = arg_default
+        kwargs[var.name] = (
+            _build(entry.type, {}) if arg_default is None else arg_default
+        )
    else:
        kwargs[var.name] = _build(
            entry.type, dictionary.get(arg_key, arg_default)
        )

@azazel75 this is related to #24. You mentioned you expect that the build config instance to be None for missing nested configs. My initial/desired implementation is to have the "default state" of the inner config to be defined.

The proposed diff would ensure this as long as default is not None for the inner = var(InnerConfig, required=False) then the nested config will be built with its "empty" state. This will cause issues if you ever truly expect the built inner config to be None.

The other solution is to internally default config vars to some kind of custom type which is interpreted like javascript's undefined.

Your Environment

• File Config Version: 0.3.6
• Python Version: 3.7.1
• Operating System and Version: LUbuntu 18.10
• Link to your branch (if applicable):

---

❤️ Thank you!

[azazel75]

Yes, in this case It seems to me that None is an interesting value. I'm using file_config in a way where my application wants to know if a configuration subsection was omitted altogether and in such a case it would avoid doing the setup for that particular subsystem, which a different state than the default configuration for that subsection

[stephen-bunn]

Interesting. One other way of handling this would be to remove default from the "default" kwargs for var. Instead we would use **kwargs. This way I can be sure whether or not the user defined a default value or not (through kwargs.pop).

In this case I would assume that:

• If the user defines a default value for a nested config, then a config missing that var should use the given default value.
• If the user does not define a default value for a nested config, then the config should be built from the empty state of the nested config type.

The cons of this approach would be:

1. The function signature for var would no longer contain default (more implicit rather than explicit)
2. It would require you to specify default=None on vars you would want to come back as None when they don't exist in the serialized content (more explicit rather than implicit)

[stephen-bunn]

Looking at this, I'm kinda leaning away from removing the default kwarg from the var method signature. This would have to cascade down through _ConfigEntry and in other various methods of the library...

Perhaps creating a new type UNDEFINED = typing.NewType("undefined", type(None)) would work?.

This would make the above diff closer to this...

elif is_config_type(entry.type):
    if arg_key not in dictionary:
-        kwargs[var.name] = arg_default
+        kwargs[var.name] = (
+            _build(entry.type, {}) if arg_default is UNDEFINED else arg_default
+        )
    else:
        kwargs[var.name] = _build(
            entry.type, dictionary.get(arg_key, arg_default)
        )

It would also change the var method signature to this...

def var(
    type=None,  # noqa
-   defalut=None,
+   default=UNDEFINED,
    name=None,
    title=None,
    description=None,
    required=True,
    examples=None,
    encoder=None,
    decoder=None,
    min=None,  # noqa
    max=None,  # noqa
    unique=None,
    contains=None,
    **kwargs,
):

Which is kinda messy (and not really Pythonic) but much easier to implement and maintain IMO.

[stephen-bunn]

Potential changes involving a new TypeVar("undefined") can be seen here. I'm still unsure of the implementation as anything that doesn't define a default and is missing from the content being loaded will come through as undefined now (instead of None)...

I added a utils.is_undefined to kinda keep it all together. But I'm not incredibly happy about the structure quite yet. Might take a look at doing it differently by passing the type as the default.

[azazel75]

It seems good to me but I share your concerns about it being the ultimate default value... the first thing that comes to mind is the fact that an undefined value should not be serialized, but instead it's key should be stripped from the containing dictionary (at serialization time), for example.

[stephen-bunn]

I think I am going to go in a different direction honestly. I've settled to just let the default remain None as changing that causes a lot of confusion both in development and usage of the library.

Instead I think I'll go with this logic for building nested configs...

elif is_config_type(entry.type):
    if arg_key not in dictionary:
        # if the default value for a nested config is the nested config class
        # then build the empty state of the nested config
        if is_config_type(arg_default) and entry.type == arg_default:
            kwargs[var.name] = _build(entry.type, {})
        else:
            kwargs[var.name] = arg_default

This way, when loading nested configs, only configs that specify that the var default is the same nested config will be loaded with the empty state of that nested config.

@file_config.config
class ParentConfig(object):
    @file_config.config
    class ChildConfig(object):
        bar = var(str, default="Default", required=False)

    foo = var(str, default="Default", required=False)
    bar = var(int)
    child = var(ChildConfig, default=ChildConfig, required=False)

So child in this case would be the new way to define loading the empty state of the ChildConfig as the default.

config_2 = ParentConfig.loads_json('{"foo": "Testing", "bar": 1}')
print(config_2)
# ParentConfig(foo='Testing', bar=1, child=ParentConfig.ChildConfig(bar='Default'))

[stephen-bunn]

Changes released in 0.3.7