Fixing TOML logging configuration #2984
Conversation
|
For convenience here is a link to my initial bug-report of this issue (#2879). This solution is better then the simple one I suggested in the original issue. I would very much want this merged. Thank you for contributing. Edit: The freezing function goes through lists and tuples and freezes all items as well. If we have a tuple/list of FrozenDicts they will not thaw in the current implementation. Not sure if this is important for the logging setup scenario, but it will hurt someone somewhere down the line. Here is a simple example of what I mean: initial = {'x': [{'a':'a', 'b':'b'}]}
frozen = recursively_freeze(initial)
frozen['x'][0]['a'] = 'hello' # This fails, as it should!
thawed = recursively_thaw(frozen)
thawed['x'][0]['a'] = 'hello' # This shuold work but does not.I suggest we add a check for tuples and recursively convert them to lists, something like this: def recursively_thaw(value):
"""
Recursively walks the given ``FrozenOrderedDict``, converting all ``FrozenOrderedDict`` objects into ``dict``. Tuples will be converted to lists, even if they were initially tuples when converting to ``FrozenOrderedDict``
"""
if isinstance(value, FrozenOrderedDict):
value = dict(value)
if isinstance(value, dict):
return {k: recursively_thaw(v) for k, v in value.items()}
elif isinstance(value, tuple):
return [recursively_thaw(item) for item in value]
return value |
|
Great catch @hirolau! I just pushed up another commit that fixes the issue. |
|
One more very very small thing. When I read the function the first time to added the line for tuples my mind read the conditions as elif instead of if. This was because I just assumed it would be a more pure recursive function, and my mind has a habit of assuming things. Thus my first attempt to change the function was a failure and I had to go back and debug. If I were to write this function I would do it like this with a single condition so the mind can be sure only one statement is run per recursion. I am fine with either implementation, but if you like my take better feel free to change it. def recursively_thaw(value):
"""
Recursively walks the given ``FrozenOrderedDict``, converting all ``FrozenOrderedDict`` objects into ``dict``.
Tuples will be converted to lists, even if they were initially tuples when converting to ``FrozenOrderedDict``.
"""
if isinstance(value, FrozenOrderedDict):
return recursively_thaw(dict(value))
elif isinstance(value, dict):
return {key: recursively_thaw(val) for key, val in value.items()}
elif isinstance(value, tuple):
return [recursively_thaw(item) for item in value]
else:
return value |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If closed, you may revisit when your time allows and reopen! Thank you for your contributions. |
Description
Using the
loggingsection of a TOML configuration file is currently not working, because theloggingmodule attempts to modify the given configuration dictionary in-place, and all parsed TOML configuration sections are converted to the immutableFrozenOrderedDictafter being read. This was originally observed on python version 3.6This change "thaws" the
FrozenOrderedDictbefore passing it into theloggingmodule.Motivation and Context
I was trying to use a TOML configuration file for logging, as shown in https://github.com/spotify/luigi/blob/master/examples/config.toml
however, trying to use it resulted in the following error