Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config_tree.put() mutates the parameter #105

Open
movermeyer opened this issue Oct 25, 2016 · 3 comments
Open

config_tree.put() mutates the parameter #105

movermeyer opened this issue Oct 25, 2016 · 3 comments

Comments

@movermeyer
Copy link
Contributor

Similarly to #95, config_tree.put() also mutates the input parameter.

from pyhocon.config_tree import ConfigTree
#This code is a parameterized version of the test "test_config_list"

value1 = [4, 5]
value2 = [6, 7]
value3 = [8, 9]

config_tree = ConfigTree()

config_tree.put("a.b.c", value1)
assert config_tree.get("a.b.c") == value1

config_tree.put("a.b.c", value2)
assert config_tree.get("a.b.c") == value2

config_tree.put("a.b.c", value3, True)
assert config_tree.get("a.b.c") == (value2 + value3) #Fails!?!
#value2 == [6, 7, 8, 9], since it was mutated.

This is surprising, as I would expect it to not be mutated.

@aalba6675
Copy link
Contributor

aalba6675 commented Jan 11, 2018

The merge_configs() method is merging the top config into the base confiig and returning the latter.

RFC: for #95 and #105: how about using deepcopy in with_fallback? Smoke testing the gist above and #95 are working.

I'll leave this comment for a while before a PR.

import copy
#### way down 
    def with_fallback(self, config):
        """
        return a new config with fallback on config
        :param config: config or filename of the config to fallback on
        :return: new config with fallback on config
        """
        if isinstance(config, ConfigTree):
            config = copy.deepcopy(config)
            result = ConfigTree.merge_configs(config, copy.deepcopy(self))
        else:
            from . import ConfigFactory
            result = ConfigTree.merge_configs(ConfigFactory.parse_file(config, resolve=False), copy.deepcopy(self))

        from . import ConfigParser
        ConfigParser.resolve_substitutions(result)
        return result

@aalba6675
Copy link
Contributor

aalba6675 commented Jan 11, 2018

Smoketest #105:

>>> from pyhocon.config_tree import ConfigTree as CT
>>> config_tree = CT()
>>> 
>>> config_tree.put("a.b.c", value1)
>>> assert config_tree.get("a.b.c") == value1
>>> 
>>> config_tree.put("a.b.c", value2)
>>> assert config_tree.get("a.b.c") == value2
>>> 
>>> config_tree.put("a.b.c", value3, True)
>>> assert config_tree.get("a.b.c") == (value2 + value3)
>>> 

@aalba6675
Copy link
Contributor

Return a new object; don't mutate object and argument in config2.with_fallback(config1)

Smoketest #95:

>>> 
>>> from pyhocon.config_parser import ConfigParser as CP, ConfigFactory as CF
>>> 
>>> config1 = CF.parse_string('''
...     string = abc
... ''')
>>> 
>>> config2 = CF.parse_string('''
...     string = ${string}def
... ''', resolve=False)
>>> 
>>> ret = config2.with_fallback(config1)
>>> ret
ConfigTree([('string', 'abcdef')])
>>> config1
ConfigTree([('string', 'abc')])
>>> config2
ConfigTree([('string', [ConfigValues: [ConfigSubstitution: string],def])])
>>> 
>>> ret is config2
False
>>> ret is config1
False
>>> 

@aalba6675 aalba6675 mentioned this issue Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants