Merge pull request #672 from lindsay-stevens/pyxform-602

602: preserve order of columns when building secondary instance

pyxform/survey.py

@@ -296,7 +296,7 @@ def _generate_static_instances(list_name, choice_list) -> InstanceInfo:
                 itext_id = "-".join([list_name, str(idx)])
                 choice_element_list.append(node("itextId", itext_id))
 
-            for name, value in sorted(choice.items()):
+            for name, value in choice.items():
                 if isinstance(value, str) and name != "label":
                     choice_element_list.append(node(name, str(value)))
                 if (

pyxform/utils.py

@@ -80,10 +80,8 @@ def node(*args, **kwargs) -> DetachableElement:
     assert len(unicode_args) <= 1
     parsed_string = False
 
-    # Convert the kwargs xml attribute dictionary to a xml.dom.minidom.Element. Sort the
-    # attributes to guarantee a consistent order across Python versions.
-    # See pyxform_test_case.reorder_attributes for details.
-    for k, v in iter(sorted(kwargs.items())):
+    # Convert the kwargs xml attribute dictionary to a xml.dom.minidom.Element.
+    for k, v in iter(kwargs.items()):
         if k in blocked_attributes:
             continue
         if k == "toParseString":

pyxform/xls2json.py

@@ -59,19 +59,22 @@ def merge_dicts(dict_a, dict_b, default_key="default"):
     if dict_b is None or dict_b == {}:
         return dict_a
 
-    if type(dict_a) is not dict:
+    if not isinstance(dict_a, dict):
         if default_key in dict_b:
             return dict_b
         dict_a = {default_key: dict_a}
-    if type(dict_b) is not dict:
+    if not isinstance(dict_b, dict):
         if default_key in dict_a:
             return dict_a
         dict_b = {default_key: dict_b}
 
-    all_keys = set(dict_a.keys()).union(set(dict_b.keys()))
+    # Union keys but retain order (as opposed to set()), preferencing dict_a then dict_b.
+    # E.g. {"a": 1, "b": 2} + {"c": 3, "a": 4} -> {"a": None, "b": None, "c": None}
+    all_keys = {k: None for k in dict_a.keys()}
+    all_keys.update({k: None for k in dict_b.keys()})
 
     out_dict = dict()
-    for key in all_keys:
+    for key in all_keys.keys():
         out_dict[key] = merge_dicts(dict_a.get(key), dict_b.get(key), default_key)
     return out_dict
 

tests/fixtures/strings.ini

@@ -11,8 +11,6 @@ test_simple_integer_question_type_multilingual_control = <input ref="/test/integ
 test_simple_integer_question_type_multilingual_binding = <bind nodeset="/test/integer_q" type="int"/>
 test_simple_date_question_type_multilingual_control = <input ref="/test/date_q"><label ref="jr:itext('/test/date_q:label')"/></input>
 test_simple_date_question_type_multilingual_binding = <bind nodeset="/test/date_q" type="date"/>
-test_simple_phone_number_question_type_multilingual_control = <input ref="/test/phone_number_q"><label ref="jr:itext('/test/phone_number_q:label')"/><hint>Enter numbers only.</hint></input>
-test_simple_phone_number_question_type_multilingual_binding = <bind constraint="regex(., '^\d*$')" nodeset="/test/phone_number_q" type="string"/>
 test_simple_select_all_question_multilingual_control = <select ref="/test/select_all_q"><label ref="jr:itext('/test/select_all_q:label')"/><item><label ref="jr:itext('/test/select_all_q/f:label')"/><value>f</value></item><item><label ref="jr:itext('/test/select_all_q/g:label')"/><value>g</value></item><item><label ref="jr:itext('/test/select_all_q/h:label')"/><value>h</value></item></select>
 test_simple_select_all_question_multilingual_binding = <bind nodeset="/test/select_all_q" type="string"/>
 test_simple_decimal_question_multilingual_control = <input ref="/test/decimal_q"><label ref="jr:itext('/test/decimal_q:label')"/></input>

tests/j2x_question_tests.py

@@ -154,20 +154,31 @@ def test_simple_phone_number_question_type_multilingual(self):
             "name": "phone_number_q",
         }
 
-        expected_phone_number_control_xml = self.config.get(
-            self.cls_name, "test_simple_phone_number_question_type_multilingual_control"
-        )
-
-        expected_phone_number_binding_xml = self.config.get(
-            self.cls_name, "test_simple_phone_number_question_type_multilingual_binding"
-        )
-
         q = create_survey_element_from_dict(simple_phone_number_question)
         self.s.add_child(q)
-        self.assertEqual(ctw(q.xml_control()), expected_phone_number_control_xml)
 
-        if TESTING_BINDINGS:
-            self.assertEqual(ctw(q.xml_bindings()), expected_phone_number_binding_xml)
+        # Inspect XML Control
+        observed = q.xml_control()
+        self.assertEqual("input", observed.nodeName)
+        self.assertEqual("/test/phone_number_q", observed.attributes["ref"].nodeValue)
+        observed_label = observed.childNodes[0]
+        self.assertEqual("label", observed_label.nodeName)
+        self.assertEqual(
+            "jr:itext('/test/phone_number_q:label')",
+            observed_label.attributes["ref"].nodeValue,
+        )
+        observed_hint = observed.childNodes[1]
+        self.assertEqual("hint", observed_hint.nodeName)
+        self.assertEqual("Enter numbers only.", observed_hint.childNodes[0].nodeValue)
+
+        # Inspect XML Binding
+        expected = {
+            "nodeset": "/test/phone_number_q",
+            "type": "string",
+            "constraint": r"regex(., '^\d*$')",
+        }
+        observed = {k: v for k, v in q.xml_bindings()[0].attributes.items()}
+        self.assertDictEqual(expected, observed)
 
     def test_simple_select_all_question_multilingual(self):
         """

tests/test_choices_sheet.py

@@ -106,3 +106,32 @@ def test_choices_without_labels__for_dynamic_selects__allowed_by_pyxform(self):
                 """,
             ],
         )
+
+    def test_choices_extra_columns_output_order_matches_xlsform(self):
+        """Should find that element order matches column order."""
+        md = """
+        | survey   |                    |      |       |
+        |          | type               | name | label |
+        |          | select_one choices | a    | A     |
+        | choices  |                    |      |       |
+        |          | list_name          | name | label | geometry                 |
+        |          | choices            | 1    |       | 46.5841618 7.0801379 0 0 |
+        |          | choices            | 2    |       | 35.8805082 76.515057 0 0 |
+        """
+        self.assertPyxformXform(
+            md=md,
+            xml__xpath_contains=[
+                """
+                /h:html/h:head/x:model/x:instance[@id='choices']/x:root/x:item[
+                  ./x:name[position() = 1 and text() = '1']
+                  and ./x:geometry[position() = 2]
+                ]
+                """
+                """
+                /h:html/h:head/x:model/x:instance[@id='choices']/x:root/x:item[
+                  ./x:name[position() = 1 and text() = '2']
+                  and ./x:geometry[position() = 2]
+                ]
+                """
+            ],
+        )

tests/test_external_instances.py

@@ -8,6 +8,7 @@
 
 from pyxform.errors import PyXFormError
 from tests.pyxform_test_case import PyxformTestCase, PyxformTestError
+from tests.xpath_helpers.choices import xpc
 
 
 class ExternalInstanceTests(PyxformTestCase):
@@ -229,21 +230,10 @@ def test_can__use_all_types_together_with_unique_ids(self):
                 '<instance id="city1" src="jr://file/city1.xml"/>',
                 '<instance id="cities" src="jr://file-csv/cities.csv"/>',
                 '<instance id="fruits" src="jr://file-csv/fruits.csv"/>',
-                """
-      <instance id="states">
-        <root>
-          <item>
-            <label>Pass</label>
-            <name>1</name>
-          </item>
-          <item>
-            <label>Fail</label>
-            <name>2</name>
-          </item>
-        </root>
-      </instance>
-""",
             ],  # noqa
+            xml__xpath_match=[
+                xpc.model_instance_choices_label("states", (("1", "Pass"), ("2", "Fail")))
+            ],
         )
 
     def test_cannot__use_different_src_same_id__select_then_internal(self):

tests/test_whitespace.py

@@ -27,9 +27,16 @@ def test_values_without_whitespaces_are_processed_successfully(self):
             |                | id_string          | public_key | submission_url                                    |
             |                | tutorial_encrypted | MIIB       | https://odk.ona.io/random_person/submission       |
           """
-
-        survey = self.md_to_pyxform_survey(md_raw=md)
-        expected = """<submission action="https://odk.ona.io/random_person/submission" base64RsaPublicKey="MIIB" method="post"/>"""
-        xml = survey._to_pretty_xml()
-        self.assertEqual(1, xml.count(expected))
-        self.assertPyxformXform(md=md, xml__contains=expected, run_odk_validate=True)
+        self.assertPyxformXform(
+            md=md,
+            run_odk_validate=True,
+            xml__xpath_contains=[
+                """
+                /h:html/h:head/x:model/x:submission[
+                  @action='https://odk.ona.io/random_person/submission'
+                  and @method='post'
+                  and @base64RsaPublicKey='MIIB'
+                ]
+                """
+            ],
+        )

tests/validators/server.py

@@ -2,6 +2,7 @@
 import os
 import posixpath
 import threading
+import time
 from http.server import SimpleHTTPRequestHandler
 from socketserver import ThreadingTCPServer
 from urllib.parse import unquote
@@ -70,13 +71,18 @@ def __init__(self, port=8000):
             self._server_address, self._handler, bind_and_activate=False
         )
 
-    def _bind_and_activate(self):
+    def _bind_and_activate(self, tries: int = 0):
+        tries += 1
         try:
             self.httpd.server_bind()
             self.httpd.server_activate()
-        except Exception as e:
+        except OSError as e:
             self.httpd.server_close()
-            raise e
+            if 5 < tries:
+                raise e
+            else:
+                time.sleep(0.5)
+                self._bind_and_activate(tries=tries)
 
     def start(self):
         self._bind_and_activate()