Merge pull request #296 from mpsonntag/fixConverter

Fix VersionConverter encoding issue

LGTM

Author: achilleas-k

.travis.yml

@@ -15,10 +15,6 @@ matrix:
     - os: linux
       python: "3.6"
 
-    - os: osx
-      language: generic
-      env:
-        - OSXENV=3.5.0
     - os: osx
       language: generic
       env:

odml/tools/version_converter.py

@@ -52,6 +52,9 @@ def _parse_xml(self):
             if elem in doc:
                 doc = doc.replace(elem, val)
 
+        # Make sure encoding is present for the xml parser
+        doc = doc.encode('utf-8')
+
         # Make pretty print available by resetting format
         parser = ET.XMLParser(remove_blank_text=True)
         tree = ET.ElementTree(ET.fromstring(doc, parser))
@@ -300,7 +303,7 @@ def _handle_properties(self, root):
 
                 if value.text:
                     if main_val.text:
-                        main_val.text += ", " + value.text.strip()
+                        main_val.text += "," + value.text.strip()
                         multiple_values = True
                     else:
                         main_val.text = value.text.strip()

odml/tools/xmlparser.py

@@ -27,11 +27,15 @@
 
 
 def to_csv(val):
-    unicode_values = list(map(unicode, val))
+    # Make sure all individual values do not contain
+    # leading or trailing whitespaces.
+    unicode_values = list(map(unicode.strip, map(unicode, val)))
     stream = StringIO()
     writer = csv.writer(stream, dialect="excel")
     writer.writerow(unicode_values)
-    csv_string = stream.getvalue().strip()
+    # Strip any csv.writer added carriage return line feeds
+    # and double quotes before saving.
+    csv_string = stream.getvalue().strip().strip('"')
     if len(unicode_values) > 1:
         csv_string = "[" + csv_string + "]"
     return csv_string
@@ -40,8 +44,14 @@ def to_csv(val):
 def from_csv(value_string):
     if not value_string:
         return []
-    if value_string[0] == "[":
+    if value_string[0] == "[" and value_string[-1] == "]":
         value_string = value_string[1:-1]
+    else:
+        # This is a single string entry, any comma contained
+        # is part of the value and must not be used to
+        # split up the string.
+        return [value_string]
+
     if not value_string:
         return []
     stream = StringIO(value_string)

test/test_version_converter.py

@@ -130,7 +130,7 @@ def test_convert_odml_file(self):
         self.assertEqual(val_elems[0].find("unit"), None)
         self.assertEqual(val_elems[0].find("type"), None)
         self.assertEqual(val_elems[0].find("uncertainty"), None)
-        self.assertEqual(val_elems[0].text, "[0, 45]")
+        self.assertEqual(val_elems[0].text, "[0,45]")
         self.assertEqual(prop.find("unit").text, "deg")
         self.assertEqual(len(prop.findall("unit")), 1)
         self.assertEqual(prop.find("type").text, "int")
@@ -481,6 +481,37 @@ def test_convert_odml_file_value(self):
                       </value>
                     </property>
 
+                    <property>
+                      <value>Single, string, value, with, many, commata.<type>string</type></value>
+                      <name>testSingleString</name>
+                    </property>
+
+                    <property>
+                      <value>A<type>string</type></value>
+                      <value>B<type>string</type></value>
+                      <value>C<type>string</type></value>
+                      <name>testStringList</name>
+                    </property>
+
+                    <property>
+                      <value> Single string value with wrapping whitespace <type>string</type></value>
+                      <name>testStringWhiteSpace</name>
+                    </property>
+
+                    <property>
+                      <value> Multiple Strings <type>string</type></value>
+                      <value> with wrapping <type>string</type></value>
+                      <value> Whitespace <type>string</type></value>
+                      <name>testStringListWhiteSpace</name>
+                    </property>
+
+                    <property>
+                      <value> 1 <type>int</type></value>
+                      <value> 2 <type>int</type></value>
+                      <value> 3 <type>int</type></value>
+                      <name>testIntListWhiteSpace</name>
+                    </property>
+
                   </section>
                 </odML>
         """
@@ -490,7 +521,7 @@ def test_convert_odml_file_value(self):
         conv_doc = vc._convert(vc._parse_xml())
         root = conv_doc.getroot()
         sec = root.find("section")
-        self.assertEqual(len(sec), 9)
+        self.assertEqual(len(sec), 14)
 
         # Test single value export
         prop = sec.findall("property")[0]
@@ -500,7 +531,7 @@ def test_convert_odml_file_value(self):
         # Test multiple value export
         prop = sec.findall("property")[1]
         self.assertEqual(len(prop), 2)
-        self.assertEqual(prop.find("value").text, "[1, 2, 3]")
+        self.assertEqual(prop.find("value").text, "[1,2,3]")
 
         # Test empty value export
         prop = sec.findall("property")[2]
@@ -521,7 +552,7 @@ def test_convert_odml_file_value(self):
         # Test valid multiple Value tag export
         prop = sec.findall("property")[4]
         self.assertEqual(len(prop), 7)
-        self.assertEqual(prop.find("value").text, "[0.1, 0.2, 3]")
+        self.assertEqual(prop.find("value").text, "[0.1,0.2,3]")
         self.assertEqual(prop.find("type").text, "float")
         self.assertEqual(prop.find("uncertainty").text, "0.05")
         self.assertEqual(prop.find("unit").text, "mV")
@@ -541,6 +572,35 @@ def test_convert_odml_file_value(self):
         self.assertEqual(prop.find("name").text, "Unsupported binary value dtype replace")
         self.assertEqual(prop.find("type").text, "text")
 
+        # Test single string value with commata
+        prop = sec.findall("property")[8]
+        self.assertEqual(prop.find("name").text, "testSingleString")
+        self.assertEqual(prop.find("value").text,
+                         "Single, string, value, with, many, commata.")
+
+        # Test string list import
+        prop = sec.findall("property")[9]
+        self.assertEqual(prop.find("name").text, "testStringList")
+        self.assertEqual(prop.find("value").text, "[A,B,C]")
+
+        # Test single string values wrapping whitespace removal
+        prop = sec.findall("property")[10]
+        self.assertEqual(prop.find("name").text, "testStringWhiteSpace")
+        self.assertEqual(prop.find("value").text,
+                         "Single string value with wrapping whitespace")
+
+        # Test multiple string values with wrapping whitespace removal
+        prop = sec.findall("property")[11]
+        self.assertEqual(prop.find("name").text, "testStringListWhiteSpace")
+        self.assertEqual(prop.find("value").text,
+                         "[Multiple Strings,with wrapping,Whitespace]")
+
+        # Test multiple int values with wrapping whitespaces
+        prop = sec.findall("property")[12]
+        self.assertEqual(prop.find("name").text, "testIntListWhiteSpace")
+        self.assertEqual(prop.find("type").text, "int")
+        self.assertEqual(prop.find("value").text, "[1,2,3]")
+
     def test_parse_dict_document(self):
         # Test appending tags; not appending empty sections
         doc_dict = {'Document': {'author': 'HPL', 'sections': []}}