Repository: jbasko/configmanager

Issue:
Add Item.str_value
Because str(item) is not intuitive and easy to miss the implications of

Discussion:

Patch:
diff --git a/configmanager/__init__.py b/configmanager/__init__.py
index 9b6663e..0939e31 100644
--- a/configmanager/__init__.py
+++ b/configmanager/__init__.py
@@ -1,4 +1,4 @@
-__version__ = '1.5.1'
+__version__ = '1.5.2'
 
 from .managers import Config
 from .items import Item
diff --git a/configmanager/items.py b/configmanager/items.py
index 0ff7467..de3f8fd 100644
--- a/configmanager/items.py
+++ b/configmanager/items.py
@@ -1,6 +1,7 @@
 import copy
 
 import six
+from builtins import str
 
 from .exceptions import ConfigValueMissing
 from .base import ItemAttribute, BaseItem
@@ -44,13 +45,27 @@ class Item(BaseItem):
         if 'type' in kwargs:
             self.type = kwargs.pop('type')
         else:
+            #
             # Type guessing
+            #
             value = kwargs.get('value', not_set)
             default = kwargs.get('default', not_set)
+
+            # 'str' is from builtins package which means that
+            # it is actually a unicode string in Python 2 too.
+            type_ = None
             if value is not not_set and value is not None:
-                self.type = type(value)
+                type_ = type(value)
             elif default is not not_set and default is not None:
-                self.type = type(default)
+                type_ = type(default)
+
+            if type_:
+                if issubclass(type_, six.string_types):
+                    self.type = str
+                else:
+                    self.type = type_
+
+            # TODO default should also be stringified accordingly!
 
         self._value = not_set
         for k, v in kwargs.items():
@@ -67,6 +82,13 @@ class Item(BaseItem):
         return '<{} {} {!r}>'.format(self.__class__.__name__, self.name, value)
 
     def __str__(self):
+        return self.str_value
+
+    def __eq__(self, other):
+        return isinstance(other, self.__class__) and self.__dict__ == other.__dict__
+
+    @property
+    def str_value(self):
         if self.raw_str_value is not not_set:
             return self.raw_str_value
         if self._value is not not_set or self.default is not not_set:
@@ -74,9 +96,6 @@ class Item(BaseItem):
         else:
             return repr(self)
 
-    def __eq__(self, other):
-        return isinstance(other, self.__class__) and self.__dict__ == other.__dict__
-
     @property
     def value(self):
         """
diff --git a/configmanager/persistence.py b/configmanager/persistence.py
index 191d05e..7ea756d 100644
--- a/configmanager/persistence.py
+++ b/configmanager/persistence.py
@@ -1,5 +1,6 @@
+from builtins import str
 import configparser
-
+from io import open
 import six
 
 
@@ -27,12 +28,12 @@ class ConfigPersistenceAdapter(object):
 
     def load(self, source, as_defaults=False):
         if isinstance(source, six.string_types):
-            with open(source) as f:
+            with open(source, encoding='utf-8') as f:
                 self._rw.load_config_from_file(self._config, f, as_defaults=as_defaults)
 
         elif isinstance(source, (list, tuple)):
             for s in source:
-                with open(s) as f:
+                with open(s, encoding='utf-8') as f:
                     self._rw.load_config_from_file(self._config, f, as_defaults=as_defaults)
 
         else:
@@ -43,7 +44,7 @@ class ConfigPersistenceAdapter(object):
 
     def dump(self, destination, with_defaults=False):
         if isinstance(destination, six.string_types):
-            with open(destination, 'w') as f:
+            with open(destination, 'w', encoding='utf-8') as f:
                 self._rw.dump_config_to_file(self._config, f, with_defaults=with_defaults)
         else:
             self._rw.dump_config_to_file(self._config, destination, with_defaults=with_defaults)
@@ -60,10 +61,20 @@ class JsonReaderWriter(ConfigReaderWriter):
         self.json = json
 
     def dump_config_to_file(self, config, file_obj, with_defaults=False, **kwargs):
-        self.json.dump(config.to_dict(with_defaults=with_defaults), file_obj, **kwargs)
+        # See comment in JsonReaderWriter.dump_config_to_string
+        file_obj.write(self.dump_config_to_string(config, with_defaults=with_defaults), **kwargs)
 
     def dump_config_to_string(self, config, with_defaults=False, **kwargs):
-        return self.json.dumps(config.to_dict(with_defaults=with_defaults), **kwargs)
+        # There is some inconsistent behaviour in Python 2's json.dump as described here:
+        # http://stackoverflow.com/a/36008538/38611
+        # and io.open which we use for file opening is very strict and fails if
+        # the string we are trying to write is not unicode in Python 2
+        # because we open files with encoding=utf-8.
+        result = self.json.dumps(config.to_dict(with_defaults=with_defaults), ensure_ascii=False, **kwargs)
+        if not isinstance(result, str):
+            return str(result)
+        else:
+            return result
 
     def load_config_from_file(self, config, file_obj, as_defaults=False, **kwargs):
         config.read_dict(self.json.load(file_obj, **kwargs), as_defaults=as_defaults)
@@ -119,6 +130,17 @@ class ConfigParserReaderWriter(ConfigReaderWriter):
         self._load_config_from_config_parser(config, cp, as_defaults=as_defaults)
 
     def _load_config_from_config_parser(self, config, cp, as_defaults=False):
+        for option, value in cp.defaults().items():
+            if as_defaults:
+                if option not in config:
+                    config.cm__add_item(option, config.cm__create_item(option, default=value))
+                else:
+                    config[option].default = value
+            else:
+                if option not in config:
+                    continue
+                config[option].value = value
+
         for section in cp.sections():
             for option in cp.options(section):
                 value = cp.get(section, option)
@@ -149,9 +171,9 @@ class ConfigParserReaderWriter(ConfigReaderWriter):
             if len(item_path) == 2:
                 section, option = item_path
             else:
-                section = 'DEFAULT'
+                section = cp.default_section
                 option = item_path[0]
 
-            if not cp.has_section(section):
+            if not cp.has_section(section) and section != cp.default_section:
                 cp.add_section(section)
-            cp.set(section, option, str(item))
+            cp.set(section, option, item.str_value)
diff --git a/configmanager/utils.py b/configmanager/utils.py
index 6c884ee..754c166 100644
--- a/configmanager/utils.py
+++ b/configmanager/utils.py
@@ -1,3 +1,6 @@
+from builtins import str
+
+
 class _NotSet(object):
     instance = None
 
diff --git a/requirements.txt b/requirements.txt
index 5e29590..e1b0220 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,6 +2,7 @@
 # Real dependencies
 #
 six
+future
 
 #
 # Potentially optional dependencies
diff --git a/setup.py b/setup.py
index 15cdf05..f872b89 100644
--- a/setup.py
+++ b/setup.py
@@ -26,7 +26,7 @@ setup(
     description='Self-conscious items of configuration in Python',
     long_description=read('README.rst'),
     packages=find_packages(),
-    install_requires=['six', 'configparser'],
+    install_requires=['six', 'future', 'configparser'],
     classifiers=[
         'Development Status :: 4 - Beta',
         'Intended Audience :: Developers',


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 562ab1c..a51c096 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -1,5 +1,9 @@
+# -*- coding: utf-8 -*-
+
 import pytest
 
+from builtins import str
+
 from configmanager import Config, Item
 
 
@@ -442,3 +446,17 @@ def test_section_knows_its_alias():
     config.uploads.db = Config({'connection': {'user': 'root'}})
     assert config.uploads.db.alias == 'db'
     assert config.uploads.db.connection.alias == 'connection'
+
+
+def test_config_item_value_can_be_unicode_str(tmpdir):
+    config1 = Config({'greeting': u'Hello, {name}', 'name': u'Anonymous'})
+    config1.name.value = u'Jānis Bērziņš'
+    assert config1.name.type is str
+
+    path = tmpdir.join('config.ini').strpath
+    config1.configparser.dump(path, with_defaults=True)
+
+    config2 = Config({'greeting': '', 'name': ''})
+    config2.configparser.load(path)
+    assert config2.name.value == u'Jānis Bērziņš'
+    assert config1.to_dict(with_defaults=True) == config2.to_dict(with_defaults=True)
diff --git a/tests/test_configparser.py b/tests/test_configparser.py
index 0121cc3..5e3bd27 100644
--- a/tests/test_configparser.py
+++ b/tests/test_configparser.py
@@ -1,3 +1,4 @@
+import collections
 import pytest
 
 from configmanager import Config, Item
@@ -263,3 +264,22 @@ def test_write_string_returns_valid_configparser_string():
     config = Config({'db': {'user': 'root'}})
     assert config.configparser.dumps() == ''
     assert config.configparser.dumps(with_defaults=True) == '[db]\nuser = root\n\n'
+
+
+def test_writes_to_and_reads_from_default_section_transparently(tmpdir):
+    config_ini = tmpdir.join('config.ini').strpath
+
+    config1 = Config(collections.OrderedDict([('greeting', 'Hello'), ('name', 'World')]))
+    config1.configparser.dump(config_ini, with_defaults=True)
+
+    with open(config_ini) as f:
+        assert f.read() == (
+            '[DEFAULT]\n'
+            'greeting = Hello\n'
+            'name = World\n\n'
+        )
+
+    config2 = Config()
+    config2.configparser.load(config_ini, as_defaults=True)
+
+    assert config1.to_dict() == config2.to_dict() == {'greeting': 'Hello', 'name': 'World'}
diff --git a/tests/test_item.py b/tests/test_item.py
index 4df094b..1b87cf0 100644
--- a/tests/test_item.py
+++ b/tests/test_item.py
@@ -1,5 +1,8 @@
+# -*- coding: utf-8 -*-
+
 import pytest
 import six
+from builtins import str
 
 from configmanager.utils import not_set
 from configmanager import Item, ConfigValueMissing
@@ -55,28 +58,28 @@ def test_int_value():
 
 def test_raw_str_value_is_reset_on_reset():
     c = Item('a', type=int, default=25)
-    assert str(c) == '25'
+    assert c.str_value == '25'
 
     c.value = '23'
-    assert str(c) == '23'
+    assert c.str_value == '23'
 
     c.reset()
-    assert str(c) == '25'
+    assert c.str_value == '25'
 
 
 def test_raw_str_value_is_reset_on_non_str_value_set():
     c = Item('a', type=int, default=25)
     c.value = '23'
-    assert str(c) == '23'
+    assert c.str_value == '23'
 
     c.value = 25
-    assert str(c) == '25'
+    assert c.str_value == '25'
 
     c.value = 24
-    assert str(c) == '24'
+    assert c.str_value == '24'
 
     c.value = '22'
-    assert str(c) == '22'
+    assert c.str_value == '22'
 
 
 def test_bool_of_value():
@@ -108,16 +111,16 @@ def test_repr_makes_clear_name_and_value():
 
 def test_str_and_repr_of_not_set_value_should_not_fail():
     c = Item('a')
-    assert str(c) == '<Item a <NotSet>>'
+    assert c.str_value == '<Item a <NotSet>>'
     assert repr(c) == '<Item a <NotSet>>'
 
 
 def test_bool_str_is_a_str():
     c = Item('a', type=bool)
-    assert isinstance(str(c), six.string_types)
+    assert isinstance(c.str_value, six.string_types)
 
     c.value = True
-    assert isinstance(str(c), six.string_types)
+    assert isinstance(c.str_value, six.string_types)
 
 
 def test_bool_config_preserves_raw_str_value_used_to_set_it():
@@ -125,27 +128,27 @@ def test_bool_config_preserves_raw_str_value_used_to_set_it():
     assert c.value is False
 
     assert not c.value
-    assert str(c) == 'False'
+    assert c.str_value == 'False'
     assert c.value is False
 
     c.value = 'False'
     assert not c.value
-    assert str(c) == 'False'
+    assert c.str_value == 'False'
     assert c.value is False
 
     c.value = 'no'
     assert not c.value
-    assert str(c) == 'no'
+    assert c.str_value == 'no'
     assert c.value is False
 
     c.value = '0'
     assert not c.value
-    assert str(c) == '0'
+    assert c.str_value == '0'
     assert c.value is False
 
     c.value = '1'
     assert c.value
-    assert str(c) == '1'
+    assert c.str_value == '1'
     assert c.value is True
 
     c.reset()
@@ -153,7 +156,7 @@ def test_bool_config_preserves_raw_str_value_used_to_set_it():
     assert c.value is False
 
     c.value = 'yes'
-    assert str(c) == 'yes'
+    assert c.str_value == 'yes'
     assert c.value is True
 
 
@@ -265,6 +268,20 @@ def test_type_is_guessed_either_from_default_or_value():
     c = Item()
     assert c.type is str
 
+    c = Item(value='haha')
+    assert c.type is str
+
+    c = Item(value=u'hāhā')
+    assert c.type is str
+    assert c.value == u'hāhā'
+    assert c.str_value == u'hāhā'
+
+    c = Item(default='haha')
+    assert c.type is str
+
+    c = Item(default=u'haha')
+    assert c.type is str
+
     d = Item(default=5)
     assert d.type is int
 
@@ -295,3 +312,12 @@ def test_item_value_is_not_deep_copied_on_value_request():
 
     assert c.value == ['c', 'd', 'e']
     assert c.default == ['a', 'b']
+
+
+def test_item_value_can_be_unicode_str():
+    c = Item(default=u'Jānis Bērziņš')
+    assert c.str_value == u'Jānis Bērziņš'
+
+    c.value = u'Pēteris Liepiņš'
+    assert c.str_value == u'Pēteris Liepiņš'
+    assert c.default == u'Jānis Bērziņš'
diff --git a/tests/test_item_get_set.py b/tests/test_item_get_set.py
index ac3d80a..edd07d1 100644
--- a/tests/test_item_get_set.py
+++ b/tests/test_item_get_set.py
@@ -34,9 +34,9 @@ def test_get_returns_default_value_when_available():
 
 def test_get_returns_value_when_value_and_default_available():
     assert Item(default='a', value=None).get() is None
-    assert Item(default='a', value='b').get() is 'b'
+    assert Item(default='a', value='b').get() == 'b'
     assert Item(default=None, value=None).get(True) is None
-    assert Item(default='a', value='b').get('c') is 'b'
+    assert Item(default='a', value='b').get('c') == 'b'
 
 
 def test_value_calls_get_so_users_can_extend_item_class_by_overriding_just_get():