Repository: silx-kit/silx
Issue: ConfigDict can't handle empty strings, strings containing "%" and None

Reading back empty strings causes a crash on all version of python (easy to fix).

Having a "%" character inside a string works with python2 but not python3 (we probably need to escape `%` at some stage).

Reading back `None` yields the string `"None"` (probably easy to fix).


Changed files:
silx/io/test/test_dictdump.py, silx/io/configdict.py

diff --git a/silx/io/configdict.py b/silx/io/configdict.py
index 603ee15c8..7281d354f 100644
--- a/silx/io/configdict.py
+++ b/silx/io/configdict.py
@@ -86,9 +86,10 @@ Classes:
 
 __author__ = ["E. Papillon", "V.A. Sole", "P. Knobel"]
 __license__ = "MIT"
-__date__ = "26/04/2016"
+__date__ = "28/04/2016"
 
 import numpy
+import re
 import sys
 if sys.version < '3.0':
     import ConfigParser as configparser
@@ -140,6 +141,8 @@ def _parse_simple_types(sstr):
             try:
                 return _boolean(sstr)
             except ValueError:
+                if sstr.strip() == "None":
+                    return None
                 # un-escape string
                 sstr = sstr.lstrip("\\\\")
                 # un-escape commas
@@ -164,6 +167,9 @@ def _parse_container(sstr):
     """
     sstr = sstr.strip()
 
+    if not sstr:
+        raise ValueError
+
     if sstr.find(',') == -1:
         # it is not a list
         if (sstr[0] == '[') and (sstr[-1] == ']'):
@@ -418,15 +424,27 @@ class ConfigDict(dict):
             fp.close()
 
     def _escape_str(self, sstr):
-        """Escape strings and individual commas with a ``\\`` character.
+        """Escape strings and special characters in strings with a ``\\``
+        character.
 
         This way, we ensure these strings cannot be interpreted as a numeric
-        or boolean types, and commas in strings are not interpreted as list
-        items separators..
+        or boolean types and commas in strings are not interpreted as list
+        items separators. We also escape ``%`` when it is not followed by a
+        ``(``, as required by ``configparser`` because ``%`` is used in
+        the interpolation syntax
+        (https://docs.python.org/3/library/configparser.html#interpolation-of-values).
         """
-        sstr = "\\\\" + sstr
+        non_str = r'^([0-9]+|[0-9]*\\.[0-9]*|none|false|true|on|off|yes|no)$'
+        if re.match(non_str, sstr.lower()):
+            sstr = "\\\\" + sstr
         # Escape commas
         sstr = sstr.replace(",", "\\,")
+
+        if sys.version > '3.0':
+            # Escape % except in "%%" and "%("
+            # argparse will handle converting %% back to %
+            sstr = re.sub(r'%([^%\\(])', r'%%\\1', sstr)
+
         return sstr
 
     def __write(self, fp, ddict, secthead=None):
@@ -461,7 +479,7 @@ class ConfigDict(dict):
                 fp.write('%s = %s\\n' % (key, ddict[key]))
 
         for key in strkey:
-            fp.write('%s = \\%s\\n' % (key, self._escape_str(ddict[key])))
+            fp.write('%s = %s\\n' % (key, self._escape_str(ddict[key])))
 
         for key in listkey:
             fp.write('%s = ' % key)
diff --git a/silx/io/test/test_dictdump.py b/silx/io/test/test_dictdump.py
index 707cb8da7..7f0059f46 100644
--- a/silx/io/test/test_dictdump.py
+++ b/silx/io/test/test_dictdump.py
@@ -25,7 +25,7 @@
 
 __authors__ = ["P. Knobel"]
 __license__ = "MIT"
-__date__ = "19/04/2016"
+__date__ = "28/04/2016"
 
 import h5py
 import numpy
@@ -115,14 +115,21 @@ class TestDictToIni(unittest.TestCase):
         os.rmdir(self.dir_path)
 
     def testConfigDictIO(self):
+        """Ensure values and types of data is preserved when dictionary is
+        written to file and read back."""
         testdict = {
             'simple_types': {
                 'float': 1.0,
                 'int': 1,
-                'string': 'Hello World',
+                'percent string': '5 % is too much',
+                'backslash string': 'i can use \\\\',
+                'empty_string': '',
+                'nonestring': 'None',
+                'nonetype': None,
+                'interpstring': 'interpolation: %(percent string)s',
             },
             'containers': {
-                'list': [-1, 'string', 3.0, False],
+                'list': [-1, 'string', 3.0, False, None],
                 'array': numpy.array([1.0, 2.0, 3.0]),
                 'dict': {
                     'key1': 'Hello World',
@@ -143,6 +150,11 @@ class TestDictToIni(unittest.TestCase):
         self.assertTrue(len(readkeys) == len(testdictkeys),
                         "Number of read keys not equal")
 
+        self.assertEqual(readinstance['simple_types']["interpstring"],
+                         "interpolation: 5 % is too much")
+
+        testdict['simple_types']["interpstring"] = "interpolation: 5 % is too much"
+
         for key in testdict["simple_types"]:
             original = testdict['simple_types'][key]
             read = readinstance['simple_types'][key]
@@ -156,8 +168,8 @@ class TestDictToIni(unittest.TestCase):
                 self.assertEqual(read.all(), original.all(),
                             "Read <%s> instead of <%s>" % (read, original))
             else:
-                self.assertEqual(read, original, 
-                                 "Read <%s> instead of <%s>" % (read, original))
+                self.assertEqual(read, original,
+                            "Read <%s> instead of <%s>" % (read, original))