From aa30abc4a80ee97a435c6a98c4737841787b49d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Thu, 25 Feb 2021 12:07:26 -0800 Subject: [PATCH 1/3] Add support for csv-external Also remove Validate runs from tests to speed up test suite --- pyxform/builder.py | 2 +- pyxform/question_type_dictionary.py | 3 + pyxform/survey.py | 3 +- ..._xmldata.py => test_external_instances.py} | 81 +++++++++++++++++-- ...=> test_external_instances_for_selects.py} | 2 +- 5 files changed, 80 insertions(+), 11 deletions(-) rename pyxform/tests_v1/{test_xmldata.py => test_external_instances.py} (87%) rename pyxform/tests_v1/{test_support_external_instances.py => test_external_instances_for_selects.py} (99%) diff --git a/pyxform/builder.py b/pyxform/builder.py index 8785bad69..ff1db1048 100644 --- a/pyxform/builder.py +++ b/pyxform/builder.py @@ -115,7 +115,7 @@ def create_survey_element_from_dict(self, d): d = self._sections[section_name] full_survey = self.create_survey_element_from_dict(d) return full_survey.children - elif d["type"] == "xml-external": + elif d["type"] in ["xml-external", "csv-external"]: return ExternalInstance(**d) else: self._save_trigger_as_setvalue_and_remove_calculate(d) diff --git a/pyxform/question_type_dictionary.py b/pyxform/question_type_dictionary.py index 9686de288..43be6cdfe 100644 --- a/pyxform/question_type_dictionary.py +++ b/pyxform/question_type_dictionary.py @@ -369,6 +369,9 @@ def generate_new_dict(): "xml-external": { # Only effect is to add an external instance. }, + "csv-external": { + # Only effect is to add an external instance. + }, "start-geopoint": { "control": {"tag": "action"}, "bind": {"type": "geopoint"}, diff --git a/pyxform/survey.py b/pyxform/survey.py index 73025a4da..fc85b6ff9 100644 --- a/pyxform/survey.py +++ b/pyxform/survey.py @@ -312,7 +312,8 @@ def _generate_static_instances(list_name, choice_list): def _generate_external_instances(element): if isinstance(element, ExternalInstance): name = element["name"] - src = "jr://file/{}.xml".format(name) + extension = element["type"].split("-")[0] + src = "jr://file/{}.{}".format(name, extension) return InstanceInfo( type="external", context="[type: {t}, name: {n}]".format( diff --git a/pyxform/tests_v1/test_xmldata.py b/pyxform/tests_v1/test_external_instances.py similarity index 87% rename from pyxform/tests_v1/test_xmldata.py rename to pyxform/tests_v1/test_external_instances.py index 3dcbc80a5..bacb0337d 100644 --- a/pyxform/tests_v1/test_xmldata.py +++ b/pyxform/tests_v1/test_external_instances.py @@ -2,7 +2,7 @@ """ Test xml-external syntax and instances generated from pulldata calls. -See also test_support_external_instances +See also test_external_instances_for_selects """ from pyxform.errors import PyXFormError from pyxform.tests_v1.pyxform_test_case import PyxformTestCase, PyxformTestError @@ -22,7 +22,19 @@ def test_can__output_single_external_xml_item(self): | | xml-external | mydata | | """, model__contains=[''], - run_odk_validate=True, + ) + + def test_can__output_single_external_csv_item(self): + """Simplest possible example to include an external instance.""" + self.assertPyxformXform( + md=""" + | survey | | | | + | | type | name | label | + | | csv-external | mydata | | + """, + model__contains=[ + '' + ], ) def test_cannot__use_same_external_xml_id_in_same_section(self): @@ -56,7 +68,21 @@ def test_can__use_unique_external_xml_in_same_section(self): '', '', ], - run_odk_validate=True, + ) + + def test_can__use_unique_external_csv_in_same_section(self): + """Two unique external instances in the same section is OK.""" + self.assertPyxformXform( + md=""" + | survey | | | | + | | type | name | label | + | | csv-external | mydata | | + | | csv-external | mydata2 | | + """, + model__contains=[ + '', + '', + ], ) def test_cannot__use_same_external_xml_id_across_groups(self): @@ -79,6 +105,23 @@ def test_cannot__use_same_external_xml_id_across_groups(self): self.assertIn("Instance names must be unique", repr(ctx.exception)) self.assertIn("The name 'mydata' was found 3 time(s)", repr(ctx.exception)) + def test_cannot__use_external_xml_and_csv_with_same_filename(self): + """Duplicate external instances anywhere raises an error.""" + with self.assertRaises(PyxformTestError) as ctx: + self.assertPyxformXform( + md=""" + | survey | | | | + | | type | name | label | + | | csv-external | mydata | | + | | begin group | g1 | | + | | xml-external | mydata | | + | | end group | g1 | | + """, + model__contains=[], + ) + self.assertIn("Instance names must be unique", repr(ctx.exception)) + self.assertIn("The name 'mydata' was found 2 time(s)", repr(ctx.exception)) + def test_can__use_unique_external_xml_across_groups(self): """Unique external instances anywhere is OK.""" self.assertPyxformXform( @@ -105,7 +148,6 @@ def test_can__use_unique_external_xml_across_groups(self): '', '', ], - run_odk_validate=True, ) def test_cannot__use_same_external_xml_id_with_mixed_types(self): @@ -133,6 +175,30 @@ def test_cannot__use_same_external_xml_id_with_mixed_types(self): ) self.assertIn("The name 'city' was found 2 time(s)", repr(ctx.exception)) + def test_can__use_same_external_csv_id_with_mixed_types(self): + """Multiple fields that require the same external instance result in a single instance declaration.""" + self.assertPyxformXform( + md=""" + | survey | | | | | + | | type | name | label | calculation | + | | begin group | g1 | | | + | | csv-external | city | | | + | | end group | g1 | | | + | | begin group | g2 | | | + | | select_one_from_file cities.csv | city | City | | + | | end group | g2 | | | + | | begin group | g3 | | | + | | select_multiple_from_file cities.csv | city | City | | + | | end group | g3 | | | + | | begin group | g4 | | | + | | calculate | city | City | pulldata('fruits', 'name', 'name', 'mango') | + | | end group | g4 | | | + """, # noqa + model__contains=[ + '', + ] + ) + def test_can__use_all_types_together_with_unique_ids(self): """Unique instances with other sources present are OK.""" self.assertPyxformXform( @@ -178,7 +244,6 @@ def test_can__use_all_types_together_with_unique_ids(self): """, ], # noqa - run_odk_validate=True, ) def test_cannot__use_different_src_same_id__select_then_internal(self): @@ -268,7 +333,7 @@ def test_can__reuse_csv__selects_then_pulldata(self): """ # noqa self.assertPyxformXform( - md=md, model__contains=[expected], run_odk_validate=True + md=md, model__contains=[expected] ) survey = self.md_to_pyxform_survey(md_raw=md) xml = survey._to_pretty_xml() @@ -288,7 +353,7 @@ def test_can__reuse_csv__pulldata_then_selects(self): """ # noqa expected = """""" # noqa self.assertPyxformXform( - md=md, model__contains=[expected], run_odk_validate=True + md=md, model__contains=[expected] ) def test_can__reuse_xml__selects_then_external(self): @@ -322,7 +387,7 @@ def test_can__reuse_xml__external_then_selects(self): """ # noqa expected = """""" # noqa self.assertPyxformXform( - md=md, model__contains=[expected], run_odk_validate=True + md=md, model__contains=[expected] ) survey = self.md_to_pyxform_survey(md_raw=md) xml = survey._to_pretty_xml() diff --git a/pyxform/tests_v1/test_support_external_instances.py b/pyxform/tests_v1/test_external_instances_for_selects.py similarity index 99% rename from pyxform/tests_v1/test_support_external_instances.py rename to pyxform/tests_v1/test_external_instances_for_selects.py index f0853542a..576bcb867 100644 --- a/pyxform/tests_v1/test_support_external_instances.py +++ b/pyxform/tests_v1/test_external_instances_for_selects.py @@ -2,7 +2,7 @@ """ Test external instance syntax -See also test_xmldata +See also test_external_instances """ from pyxform.tests_v1.pyxform_test_case import PyxformTestCase From e37b01556e5f3a2ae3067bb26f4398262fd4f059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Thu, 25 Feb 2021 14:46:56 -0800 Subject: [PATCH 2/3] Make sure all tests pass Validate --- pyxform/tests_v1/test_external_instances.py | 40 ++++++++++--------- .../test_secondary_instance_translations.py | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/pyxform/tests_v1/test_external_instances.py b/pyxform/tests_v1/test_external_instances.py index bacb0337d..e036c0a5b 100644 --- a/pyxform/tests_v1/test_external_instances.py +++ b/pyxform/tests_v1/test_external_instances.py @@ -32,9 +32,7 @@ def test_can__output_single_external_csv_item(self): | | type | name | label | | | csv-external | mydata | | """, - model__contains=[ - '' - ], + model__contains=[''], ) def test_cannot__use_same_external_xml_id_in_same_section(self): @@ -182,6 +180,7 @@ def test_can__use_same_external_csv_id_with_mixed_types(self): | survey | | | | | | | type | name | label | calculation | | | begin group | g1 | | | + | | text | foo | Foo | | | | csv-external | city | | | | | end group | g1 | | | | | begin group | g2 | | | @@ -191,12 +190,11 @@ def test_can__use_same_external_csv_id_with_mixed_types(self): | | select_multiple_from_file cities.csv | city | City | | | | end group | g3 | | | | | begin group | g4 | | | + | | text | foo | Foo | | | | calculate | city | City | pulldata('fruits', 'name', 'name', 'mango') | | | end group | g4 | | | """, # noqa - model__contains=[ - '', - ] + model__contains=['',], ) def test_can__use_all_types_together_with_unique_ids(self): @@ -332,9 +330,7 @@ def test_can__reuse_csv__selects_then_pulldata(self): expected = """ """ # noqa - self.assertPyxformXform( - md=md, model__contains=[expected] - ) + self.assertPyxformXform(md=md, model__contains=[expected]) survey = self.md_to_pyxform_survey(md_raw=md) xml = survey._to_pretty_xml() self.assertEqual(1, xml.count(expected)) @@ -352,9 +348,7 @@ def test_can__reuse_csv__pulldata_then_selects(self): | | select_one_from_file pain_locations.csv | pyear | Location of worst pain this year. | | """ # noqa expected = """""" # noqa - self.assertPyxformXform( - md=md, model__contains=[expected] - ) + self.assertPyxformXform(md=md, model__contains=[expected]) def test_can__reuse_xml__selects_then_external(self): """Re-using the same xml external data source id and URI is OK.""" @@ -386,9 +380,7 @@ def test_can__reuse_xml__external_then_selects(self): | | select_one_from_file pain_locations.xml | pyear | Location of worst pain this year. | """ # noqa expected = """""" # noqa - self.assertPyxformXform( - md=md, model__contains=[expected] - ) + self.assertPyxformXform(md=md, model__contains=[expected]) survey = self.md_to_pyxform_survey(md_raw=md) xml = survey._to_pretty_xml() self.assertEqual(1, xml.count(expected)) @@ -428,7 +420,11 @@ def test_external_instance_pulldata_readonly(self): """ node = """""" - self.assertPyxformXform(md=md, xml__contains=[node]) + self.assertPyxformXform( + md=md, + xml__contains=[node], + run_odk_validate=False, # Validate sees self references in readonly as circular but shouldn't + ) def test_external_instance_pulldata_required(self): """ @@ -441,7 +437,11 @@ def test_external_instance_pulldata_required(self): | | text | Part_ID | Participant ID | pulldata('ID', 'ParticipantID', 'ParticipantIDValue',.) | """ node = """""" - self.assertPyxformXform(md=md, xml__contains=[node], debug=False) + self.assertPyxformXform( + md=md, + xml__contains=[node], + run_odk_validate=False, # Validate sees self references in requireds as circular but shouldn't + ) def test_external_instance_pulldata_relevant(self): """ @@ -454,7 +454,11 @@ def test_external_instance_pulldata_relevant(self): | | text | Part_ID | Participant ID | pulldata('ID', 'ParticipantID', 'ParticipantIDValue',.) | """ node = """""" - self.assertPyxformXform(md=md, xml__contains=[node], debug=False) + self.assertPyxformXform( + md=md, + xml__contains=[node], + run_odk_validate=False, # Validate sees self references in relevants as circular but shouldn't + ) # This is not something that is recommended since pulldata and choice_filter both should use XPath predicates # behind the scenes but it should still be possible. diff --git a/pyxform/tests_v1/test_secondary_instance_translations.py b/pyxform/tests_v1/test_secondary_instance_translations.py index 779d32aab..9ed11bbd9 100644 --- a/pyxform/tests_v1/test_secondary_instance_translations.py +++ b/pyxform/tests_v1/test_secondary_instance_translations.py @@ -121,7 +121,7 @@ def test_select_with_choice_filter_and_translations_generates_single_translation xform_md = """ | survey | | | | | | | type | name | label | choice_filter | - | | select_one list | foo | Foo | name != " | + | | select_one list | foo | Foo | name != '' | | choices | | | list_name | name | label | image | label::French | | | list | a | A | a.jpg | Ah | From e9204c111d56c9c30dc2701f0537d9b904641e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Fri, 26 Feb 2021 15:06:46 -0800 Subject: [PATCH 3/3] Use file-csv prefix for csv external instances --- pyxform/survey.py | 3 ++- pyxform/tests_v1/test_external_instances.py | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pyxform/survey.py b/pyxform/survey.py index fc85b6ff9..7094bfa53 100644 --- a/pyxform/survey.py +++ b/pyxform/survey.py @@ -313,7 +313,8 @@ def _generate_external_instances(element): if isinstance(element, ExternalInstance): name = element["name"] extension = element["type"].split("-")[0] - src = "jr://file/{}.{}".format(name, extension) + prefix = "file-csv" if extension == "csv" else "file" + src = "jr://{}/{}.{}".format(prefix, name, extension) return InstanceInfo( type="external", context="[type: {t}, name: {n}]".format( diff --git a/pyxform/tests_v1/test_external_instances.py b/pyxform/tests_v1/test_external_instances.py index e036c0a5b..b8d527ab6 100644 --- a/pyxform/tests_v1/test_external_instances.py +++ b/pyxform/tests_v1/test_external_instances.py @@ -32,7 +32,7 @@ def test_can__output_single_external_csv_item(self): | | type | name | label | | | csv-external | mydata | | """, - model__contains=[''], + model__contains=[''], ) def test_cannot__use_same_external_xml_id_in_same_section(self): @@ -78,8 +78,8 @@ def test_can__use_unique_external_csv_in_same_section(self): | | csv-external | mydata2 | | """, model__contains=[ - '', - '', + '', + '', ], ) @@ -194,7 +194,7 @@ def test_can__use_same_external_csv_id_with_mixed_types(self): | | calculate | city | City | pulldata('fruits', 'name', 'name', 'mango') | | | end group | g4 | | | """, # noqa - model__contains=['',], + model__contains=[''], ) def test_can__use_all_types_together_with_unique_ids(self):