fix: support of or_other with selects in itemsets

- builder.py:
  - add or_other choice to survey-level choice dict
  - add type annotations
- constants.py: add new constant for "or specify other" internal suffix
- xls2json.py: add error for choice_filter + or_other. Main reason I
  suppose is that it's unknown how to filter the "other" choice so the
  user might never see it.
- xlsform_spec_test.py:
  - fix or_other expected XML to include "other" and match sorted
    element names.
  - move or_other test into this testcase since it is the same as others
    there already.
- builder_tests.py: update expected output to match or_other output.

pyxform/builder.py

@@ -5,9 +5,9 @@
 import copy
 import os
 import re
-from typing import Any, Dict, Optional
+from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
 
-from pyxform import file_utils, utils
+from pyxform import constants, file_utils, utils
 from pyxform.entities.entity_declaration import EntityDeclaration
 from pyxform.errors import PyXFormError
 from pyxform.external_instance import ExternalInstance
@@ -25,6 +25,14 @@
 from pyxform.survey import Survey
 from pyxform.xls2json import SurveyReader
 
+if TYPE_CHECKING:
+    from pyxform.survey_element import SurveyElement
+
+OR_OTHER_CHOICE = {
+    "name": "other",
+    "label": "Other",
+}
+
 
 def copy_json_dict(json_dict):
     """
@@ -88,7 +96,9 @@ def set_sections(self, sections):
         assert type(sections) == dict
         self._sections = sections
 
-    def create_survey_element_from_dict(self, d):
+    def create_survey_element_from_dict(
+        self, d: Dict[str, Any]
+    ) -> Union["SurveyElement", List["SurveyElement"]]:
         """
         Convert from a nested python dictionary/array structure (a json dict I
         call it because it corresponds directly with a json object)
@@ -143,7 +153,11 @@ def _save_trigger_as_setvalue_and_remove_calculate(self, d):
                 self.setvalues_by_triggering_ref[triggering_ref] = [(d["name"], value)]
 
     @staticmethod
-    def _create_question_from_dict(d, question_type_dictionary, add_none_option=False):
+    def _create_question_from_dict(
+        d: Dict[str, Any],
+        question_type_dictionary: Dict[str, Any],
+        add_none_option: bool = False,
+    ) -> Union[Question, List[Question]]:
         question_type_str = d["type"]
         d_copy = d.copy()
 
@@ -152,11 +166,9 @@ def _create_question_from_dict(d, question_type_dictionary, add_none_option=Fals
             SurveyElementBuilder._add_none_option_to_select_all_that_apply(d_copy)
 
         # Handle or_other on select type questions
-        or_other_str = " or specify other"
-        if question_type_str.endswith(or_other_str):
-            question_type_str = question_type_str[
-                : len(question_type_str) - len(or_other_str)
-            ]
+        or_other_len = len(constants.SELECT_OR_OTHER_SUFFIX)
+        if question_type_str.endswith(constants.SELECT_OR_OTHER_SUFFIX):
+            question_type_str = question_type_str[: len(question_type_str) - or_other_len]
             d_copy["type"] = question_type_str
             SurveyElementBuilder._add_other_option_to_multiple_choice_question(d_copy)
             return [
@@ -173,20 +185,18 @@ def _create_question_from_dict(d, question_type_dictionary, add_none_option=Fals
         # todo: clean up this spaghetti code
         d_copy["question_type_dictionary"] = question_type_dictionary
         if question_class:
-
             return question_class(**d_copy)
 
         return []
 
     @staticmethod
-    def _add_other_option_to_multiple_choice_question(d):
+    def _add_other_option_to_multiple_choice_question(d: Dict[str, Any]) -> None:
         # ideally, we'd just be pulling from children
         choice_list = d.get("choices", d.get("children", []))
         if len(choice_list) <= 0:
             raise PyXFormError("There should be choices for this question.")
-        other_choice = {"name": "other", "label": "Other"}
-        if other_choice not in choice_list:
-            choice_list.append(other_choice)
+        if OR_OTHER_CHOICE not in choice_list:
+            choice_list.append(OR_OTHER_CHOICE)
 
     @staticmethod
     def _add_none_option_to_select_all_that_apply(d_copy):
@@ -220,7 +230,7 @@ def _get_question_class(question_type_str, question_type_dictionary):
         return SurveyElementBuilder.QUESTION_CLASSES[control_tag]
 
     @staticmethod
-    def _create_specify_other_question_from_dict(d):
+    def _create_specify_other_question_from_dict(d: Dict[str, Any]) -> InputQuestion:
         kwargs = {
             "type": "text",
             "name": "%s_other" % d["name"],
@@ -242,6 +252,11 @@ def _create_section_from_dict(self, d):
             # And I hope it doesn't break something else.
             # I think the good solution would be to rewrite this class.
             survey_element = self.create_survey_element_from_dict(copy.deepcopy(child))
+            if child["type"].endswith(" or specify other"):
+                select_question = survey_element[0]
+                itemset_choices = result["choices"][select_question["itemset"]]
+                if OR_OTHER_CHOICE not in itemset_choices:
+                    itemset_choices.append(OR_OTHER_CHOICE)
             if survey_element:
                 result.add_children(survey_element)
 

pyxform/constants.py

@@ -62,6 +62,7 @@
 SELECT_ONE = "select one"
 SELECT_ONE_EXTERNAL = "select one external"
 SELECT_ALL_THAT_APPLY = "select all that apply"
+SELECT_OR_OTHER_SUFFIX = " or specify other"
 RANK = "rank"
 CHOICES = "choices"
 

pyxform/xls2json.py

@@ -1158,7 +1158,13 @@ def workbook_to_json(
 
                 specify_other_question = None
                 if parse_dict.get("specify_other") is not None:
-                    select_type += " or specify other"
+                    select_type += constants.SELECT_OR_OTHER_SUFFIX
+                    if row.get(constants.CHOICE_FILTER):
+                        msg = (
+                            ROW_FORMAT_STRING % row_number
+                            + " Choice filter not supported with or_other."
+                        )
+                        raise PyXFormError(msg)
 
                 new_json_dict = row.copy()
                 new_json_dict[constants.TYPE] = select_type

tests/builder_tests.py

@@ -129,6 +129,7 @@ def test_specify_other(self):
                 "sexes": [
                     {"label": {"English": "Male"}, "name": "male"},
                     {"label": {"English": "Female"}, "name": "female"},
+                    {"label": "Other", "name": "other"},
                 ]
             },
             "children": [
@@ -167,6 +168,7 @@ def test_specify_other(self):
             ],
         }
         self.maxDiff = None
+        observed = survey.to_json_dict()
         self.assertEqual(survey.to_json_dict(), expected_dict)
 
     def test_select_one_question_with_identical_choice_name(self):
@@ -231,6 +233,7 @@ def test_loop(self):
                         "name": "open_pit_latrine",
                     },
                     {"label": {"english": "Bucket system"}, "name": "bucket_system"},
+                    {"label": "Other", "name": "other"},
                 ]
             },
             "children": [
@@ -317,6 +320,20 @@ def test_loop(self):
                                 }
                             ],
                         },
+                        {
+                            "children": [
+                                {
+                                    "label": {
+                                        "english": "How many Other are on the premises?"
+                                    },
+                                    "name": "number",
+                                    "type": "integer",
+                                }
+                            ],
+                            "label": "Other",
+                            "name": "other",
+                            "type": "group",
+                        },
                     ],
                 },
                 {

tests/test_expected_output/or_other.xml

@@ -16,29 +16,33 @@
       <instance id="colors">
         <root>
           <item>
-            <pastel>no</pastel>
-            <name>red</name>
             <label>red</label>
+            <name>red</name>
+            <pastel>no</pastel>
           </item>
           <item>
-            <pastel>no</pastel>
-            <name>green</name>
             <label>green</label>
+            <name>green</name>
+            <pastel>no</pastel>
           </item>
           <item>
-            <pastel>no</pastel>
-            <name>blue</name>
             <label>blue</label>
+            <name>blue</name>
+            <pastel>no</pastel>
           </item>
           <item>
-            <pastel>yes</pastel>
-            <name>mauve</name>
             <label>mauve</label>
+            <name>mauve</name>
+            <pastel>yes</pastel>
           </item>
           <item>
-            <pastel>yes</pastel>
-            <name>apricot</name>
             <label>apricot</label>
+            <name>apricot</name>
+            <pastel>yes</pastel>
+          </item>
+          <item>
+            <label>Other</label>
+            <name>other</name>
           </item>
         </root>
       </instance>