diff --git a/pyxform/constants.py b/pyxform/constants.py index f44725504..6d5d570fa 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -40,7 +40,9 @@ ATTRIBUTE = "attribute" ALLOW_CHOICE_DUPLICATES = "allow_choice_duplicates" -BIND = "bind" # TODO: What should I do with the nested types? (readonly and relevant) # noqa +BIND = ( + "bind" +) # TODO: What should I do with the nested types? (readonly and relevant) # noqa MEDIA = "media" CONTROL = "control" APPEARANCE = "appearance" @@ -60,7 +62,9 @@ # XLS Specific constants LIST_NAME = "list name" CASCADING_SELECT = "cascading_select" -TABLE_LIST = "table-list" # hyphenated because it goes in appearance, and convention for appearance column is dashes # noqa +TABLE_LIST = ( + "table-list" +) # hyphenated because it goes in appearance, and convention for appearance column is dashes # noqa # The following are the possible sheet names: SURVEY = "survey" diff --git a/pyxform/tests_v1/test_fields.py b/pyxform/tests_v1/test_fields.py index dc8b7b55f..9c9c1f6b3 100644 --- a/pyxform/tests_v1/test_fields.py +++ b/pyxform/tests_v1/test_fields.py @@ -42,81 +42,97 @@ def test_duplicate_fields_diff_cases(self): error__contains=["There are more than one survey elements named 'age'"], ) - def test_duplicate_choice_list_without_settings(self): + def test_duplicate_choices_without_settings(self): self.assertPyxformXform( md=""" - | survey | | | | - | | type | name | label | - | | select_one list | S1 | s1 | - | choices | | | | - | | list name | name | label | - | | list | option a | a | - | | list | option b | b | - | | list | option b | c | + | survey | | | | + | | type | name | label | + | | select_one list | S1 | s1 | + | choices | | | | + | | list name | name | label | + | | list | a | option a | + | | list | b | option b | + | | list | b | option c | """, errored=True, error__contains=[ - "There does not seem to be a" - " `allow_choice_duplicates` column header defined" - " in your settings sheet" + "The name(s) 'b' occur(s) more than once in the 'list' choice list" ], # noqa ) - def test_duplicate_choice_list_with_wrong_setting(self): + def test_multiple_duplicate_choices_without_setting(self): self.assertPyxformXform( md=""" - | survey | | | | - | | type | name | label | - | | select_one list | S1 | s1 | - | choices | | | | - | | list name | name | label | - | | list | option a | a | - | | list | option b | b | - | | list | option b | c | - | settings | | | | + | survey | | | | + | | type | name | label | + | | select_one list | S1 | s1 | + | choices | | | | + | | list name | name | label | + | | list | a | option a | + | | list | a | option b | + | | list | b | option c | + | | list | b | option d | + """, + errored=True, + error__contains=[ + "The name(s) 'a', 'b' occur(s) more than once in the 'list' choice list." + ], # noqa + ) + + def test_duplicate_choices_with_setting_not_set_to_yes(self): + self.assertPyxformXform( + md=""" + | survey | | | | + | | type | name | label | + | | select_one list | S1 | s1 | + | choices | | | | + | | list name | name | label | + | | list | a | option a | + | | list | b | option b | + | | list | b | option c | + | settings | | | | | | id_string | allow_choice_duplicates | | | Duplicates | True | """, errored=True, error__contains=[ - "On the choices sheet the choice list name" - " 'option b' occurs more than once." + "The name(s) 'b' occur(s) more than once in the 'list' choice list" ], # noqa ) - def test_duplicate_choice_list_with_setting(self): + def test_duplicate_choices_with_allow_choice_duplicates_setting(self): md = """ - | survey | | | | - | | type | name | label | - | | select_one list | S1 | s1 | - | choices | | | | - | | list name | name | label | - | | list | option a | a | - | | list | option b | b | - | | list | option b | c | - | settings | | | | - | | id_string | allow_choice_duplicates | - | | Duplicates | Yes | + | survey | | | | + | | type | name | label | + | | select_one list | S1 | s1 | + | choices | | | | + | | list name | name | label | + | | list | a | option a | + | | list | b | option b | + | | list | b | option c | + | settings | | | | + | | id_string | allow_choice_duplicates | + | | Duplicates | Yes | """ expected = """ - - option a + + a - - option b + + b - - option b + + b """ - self.assertPyxformXform(md=md, xml__contains=[expected], run_odk_validate=True) + self.assertPyxformXform(md=md, xml__contains=[expected]) def test_choice_list_without_duplicates_is_successful(self): md = """ @@ -145,4 +161,4 @@ def test_choice_list_without_duplicates_is_successful(self): """ - self.assertPyxformXform(md=md, xml__contains=[expected], run_odk_validate=True) + self.assertPyxformXform(md=md, xml__contains=[expected]) diff --git a/pyxform/tests_v1/test_xmldata.py b/pyxform/tests_v1/test_xmldata.py index 571034405..166496cc6 100644 --- a/pyxform/tests_v1/test_xmldata.py +++ b/pyxform/tests_v1/test_xmldata.py @@ -286,7 +286,9 @@ def test_can__reuse_csv__pulldata_then_selects(self): | | select_one_from_file pain_locations.csv | pmonth | Location of worst pain this month. | | | | select_one_from_file pain_locations.csv | pyear | Location of worst pain this year. | | """ # noqa - expected = """""" # noqa + expected = ( + """""" + ) # noqa self.assertPyxformXform( md=md, model__contains=[expected], run_odk_validate=True ) @@ -320,7 +322,9 @@ def test_can__reuse_xml__external_then_selects(self): | | select_one_from_file pain_locations.xml | pmonth | Location of worst pain this month. | | | select_one_from_file pain_locations.xml | pyear | Location of worst pain this year. | """ # noqa - expected = """""" # noqa + expected = ( + """""" + ) # noqa self.assertPyxformXform( md=md, model__contains=[expected], run_odk_validate=True ) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index fca449ee6..db347a340 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -500,34 +500,28 @@ def workbook_to_json( list_name_choices = [option.get("name") for option in options] if len(list_name_choices) != len(set(list_name_choices)): duplicate_setting = settings.get("allow_choice_duplicates") - if not duplicate_setting: - raise PyXFormError( - "There does not seem to be" - " a `allow_choice_duplicates`" - " column header defined in your settings sheet." - " You must have set `allow_choice_duplicates`" - " setting in your settings sheet" - " to have duplicate choice list names" - " in your choices sheet" - ) # noqa for k, v in Counter(list_name_choices).items(): if v > 1: - if duplicate_setting and duplicate_setting.capitalize() != "Yes": - result = [ + if not duplicate_setting or duplicate_setting.capitalize() != "Yes": + choice_duplicates = [ item for item, count in Counter(list_name_choices).items() if count > 1 ] - choice_duplicates = " ".join(result) if choice_duplicates: raise PyXFormError( - "On the choices sheet the choice list name" - " '{}' occurs more than once." - " You must have set `allow_choice_duplicates`" - " setting in your settings sheet" - " for this to be a valid measure".format( - choice_duplicates + "The name(s) {} occur(s) more than once in the '{}' choice list. The options with " + "duplicate names will be impossible to identify in analysis unless a previous value in " + "a cascading select differentiates them. If this is intentional, you can set the " + "allow_choice_duplicates setting to 'yes'. Read more: https://xlsform.org/choice-names.".format( + ", ".join( + [ + "'{}'".format(dupe) + for dupe in choice_duplicates + ] + ), + list_name, ) ) # noqa