Skip to content

Commit

Permalink
Clarify duplicate choice error message
Browse files Browse the repository at this point in the history
  • Loading branch information
@lognaturel
lognaturel committed Jan 31, 2020
1 parent 8fb7934 commit 861d91d
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 67 deletions.
8 changes: 6 additions & 2 deletions pyxform/constants.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
104 changes: 60 additions & 44 deletions pyxform/tests_v1/test_fields.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """
<select1 ref="/pyxform_autotestname/S1">
<label>s1</label>
<item>
<label>a</label>
<value>option a</value>
<label>option a</label>
<value>a</value>
</item>
<item>
<label>b</label>
<value>option b</value>
<label>option b</label>
<value>b</value>
</item>
<item>
<label>c</label>
<value>option b</value>
<label>option c</label>
<value>b</value>
</item>
</select1>
"""
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 = """
Expand Down Expand Up @@ -145,4 +161,4 @@ def test_choice_list_without_duplicates_is_successful(self):
</item>
</select1>
"""
self.assertPyxformXform(md=md, xml__contains=[expected], run_odk_validate=True)
self.assertPyxformXform(md=md, xml__contains=[expected])
8 changes: 6 additions & 2 deletions pyxform/tests_v1/test_xmldata.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """<instance id="pain_locations" src="jr://file-csv/pain_locations.csv"/>""" # noqa
expected = (
"""<instance id="pain_locations" src="jr://file-csv/pain_locations.csv"/>"""
) # noqa
self.assertPyxformXform(
md=md, model__contains=[expected], run_odk_validate=True
)
Expand Down Expand Up @@ -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 = """<instance id="pain_locations" src="jr://file/pain_locations.xml"/>""" # noqa
expected = (
"""<instance id="pain_locations" src="jr://file/pain_locations.xml"/>"""
) # noqa
self.assertPyxformXform(
md=md, model__contains=[expected], run_odk_validate=True
)
Expand Down
32 changes: 13 additions & 19 deletions pyxform/xls2json.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 861d91d

Please sign in to comment.