Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

603: Always generate secondary instance for selects #614

Merged
merged 20 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
faaf52a
wip: initial impl changes and test fixes - 27 tests still broken
lindsay-stevens Jun 10, 2022
ae54917
fix: error created but not raised
lindsay-stevens Jul 20, 2022
4fa1f7d
fix: avoid output of duplicate translations, json dump, tests
lindsay-stevens Jul 20, 2022
ff31a6f
fix: lint
lindsay-stevens Jul 20, 2022
66f8838
fix: remove unused xpath expressions
lindsay-stevens Jul 20, 2022
df4e01e
fix: upgrade fixture-based test for instance_xmlns setting
lindsay-stevens Jul 21, 2022
0683836
fix: formatting
lindsay-stevens Jul 21, 2022
7807e88
fix: import error due to local name vs. package namespaced name
lindsay-stevens Jul 21, 2022
f6df5b2
fix: translation test using incorrect test name param
lindsay-stevens Apr 20, 2023
1238bd6
fix: xform2json choice filters support, minor codestyle improvements
lindsay-stevens May 1, 2023
c7caf44
add: type annotations
lindsay-stevens May 3, 2023
e5e21f5
fix: import path, add base method for non-question SurveyElement
lindsay-stevens May 3, 2023
c93d0a1
fix: remove debug output in some tests
lindsay-stevens May 4, 2023
48c6f8e
fix: update comments in _setup_choice_translations for current behaviour
lindsay-stevens May 5, 2023
42d7fd1
fix: exclude select with appearance=search() from itemset, test updates
lindsay-stevens May 5, 2023
acd7640
fix: minor correctness / off-by-one error in excel data processing
lindsay-stevens May 5, 2023
2adb257
fix: tests for new itemset output for selects, add xls2json type hints
lindsay-stevens May 5, 2023
b23d704
fix: translation tests failing due to itemset changes
lindsay-stevens May 11, 2023
c2dd549
fix: test failure on Windows due to file encoding not specified in test
lindsay-stevens May 11, 2023
751e4ea
fix: support of or_other with selects in itemsets
lindsay-stevens May 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pyxform/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
from collections import defaultdict
from datetime import datetime
from functools import lru_cache
from typing import List, Iterator, Optional
from typing import Iterator, List, Optional

import aliases

from pyxform import constants
from pyxform.constants import EXTERNAL_INSTANCE_EXTENSIONS
Expand Down Expand Up @@ -643,12 +645,13 @@ def _setup_choice_translations(name, choice_value, itext_id):
)

self._translations = defaultdict(dict) # pylint: disable=W0201
select_types = set(aliases.select.keys())
for element in self.iter_descendants():
# Skip creation of translations for choices in filtered selects
# The creation of these translations is done futher below in this
# function
parent = element.get("parent")
if parent and not parent.get("choice_filter"):
if parent is not None and parent[constants.TYPE] not in select_types:
for d in element.get_translations(self.default_language):
translation_path = d["path"]
form = "long"
Expand Down Expand Up @@ -680,7 +683,7 @@ def _setup_choice_translations(name, choice_value, itext_id):
and not has_dynamic_label(choice_list, multi_language)
):
continue
for idx, choice in zip(range(len(choice_list)), choice_list):
for idx, choice in enumerate(choice_list):
for name, choice_value in choice.items():
itext_id = "-".join([list_name, str(idx)])
if isinstance(choice_value, dict):
Expand Down
7 changes: 7 additions & 0 deletions pyxform/survey_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,13 @@ def to_json_dict(self):
result["children"] = []
for child in children:
result["children"].append(child.to_json_dict())
# Translation items with "output_context" have circular references.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output_context is a way to save the node reference for cases where there are output nodes so that ${} references can be expanded? I tried removing this scary bit of code and test_convert_toJSON_multi_language did fail. Then I set all places where output_context is set to None and some tests in test_repeat failed. I don't understand why this works, though. I thought it would remove all output_context values for all survey elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is reached by a call to survey.to_json so it shouldn't affect form conversion to XML.

The purpose of output_context is to hold a reference to a SurveyElement, so that references like ${} can be expanded. In this case we're going from "internal PyForm" structures into JSON - so I don't know what the intended JSON representation of output_context should be (maybe a copy of the node dict?). Or maybe the JSON output should have the de-referenced output somewhere (e.g. a string containing XML). It seems like it needs a separate issue to design and implement, or document.

Anyway, the problem at hand is that the node reference is a SurveyElement, which has a reference to it's parent Survey, which has references to it's child SurveyElements, which have references to the parent Survey, and so on. To break out of that loop, output_context is deleted so that survey.to_json() can return a document - it just would have the original referencing strings like My fav fruit is ${fruit}, and de-referencing those is left to the caller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is reached by a call to survey.to_json so it shouldn't affect form conversion to XML.

Aha, thank you, that's great to know.

if "_translations" in result:
for lang in result["_translations"].values():
for item in lang.values():
for form in item.values():
if callable(getattr(form, "pop", None)):
form.pop("output_context", None)
# remove any keys with empty values
for k, v in list(result.items()):
if not v:
Expand Down
20 changes: 17 additions & 3 deletions tests/pyxform_test_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ def check_content(content, expected):
nsmap_subs=final_nsmap_subs,
content_str=cstr,
)
for i in expected:
for idx, i in enumerate(expected, start=1):
if verb == "contains":
self.assertContains(cstr, i, msg_prefix=keyword)
elif verb == "excludes":
Expand All @@ -311,20 +311,23 @@ def check_content(content, expected):
content=content,
xpath=i[0],
expected=i[1],
case_num=idx,
)
elif verb == "xpath_count":
self.assert_xpath_count(
matcher_context=matcher_context,
content=content,
xpath=i[0],
expected=i[1],
case_num=idx,
)
elif verb == "xpath_match":
self.assert_xpath_count(
matcher_context=matcher_context,
content=content,
xpath=i,
expected=1,
case_num=idx,
)

return verb_str, check_content
Expand Down Expand Up @@ -442,6 +445,7 @@ def assert_xpath_exact(
content: "_Element",
xpath: str,
expected: "Set[str]",
case_num: int,
) -> None:
"""
Process an assertion for xml__xpath_exact.
Expand All @@ -454,6 +458,7 @@ def assert_xpath_exact(
:param content: XML to be examined.
:param xpath: XPath to execute.
:param expected: Expected XPath matches, as XML fragments.
:param case_num: The list position of the test case in the input.
"""
if not (isinstance(xpath, str) and isinstance(expected, set)):
msg = "Each xpath_exact requires: tuple(xpath: str, expected: Set[str])."
Expand All @@ -464,14 +469,19 @@ def assert_xpath_exact(
xpath=xpath,
for_exact=True,
)
self.assertSetEqual(set(expected), observed, matcher_context.content_str)
msg = (
f"XPath found no matches (test case {case_num}):\n{xpath}"
f"\n\nXForm content:\n{matcher_context.content_str}"
)
self.assertSetEqual(set(expected), observed, msg=msg)

def assert_xpath_count(
self,
matcher_context: "MatcherContext",
content: "_Element",
xpath: str,
expected: int,
case_num: int,
):
"""
Process an assertion for xml__xpath_count.
Expand All @@ -480,6 +490,7 @@ def assert_xpath_count(
:param content: XML to be examined.
:param xpath: XPath to execute.
:param expected: Expected count of XPath matches.
:param case_num: The list position of the test case in the input.
"""
if not (isinstance(xpath, str) and isinstance(expected, int)):
msg = "Each xpath_count requires: tuple(xpath: str, count: int)"
Expand All @@ -489,7 +500,10 @@ def assert_xpath_count(
content=content,
xpath=xpath,
)
msg = f"XPath found no matches:\n{xpath}\n\nXForm content:\n{matcher_context.content_str}"
msg = (
f"XPath found no matches (test case {case_num}):\n{xpath}"
f"\n\nXForm content:\n{matcher_context.content_str}"
)
self.assertEqual(expected, len(observed), msg=msg)


Expand Down
39 changes: 30 additions & 9 deletions tests/test_choices_sheet.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
from tests.pyxform_test_case import PyxformTestCase
from tests.xpath_helpers.choices import xp
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq


class ChoicesSheetTest(PyxformTestCase):
Expand All @@ -19,8 +20,8 @@ def test_numeric_choice_names__for_static_selects__allowed(self):
| | choices | 2 | Two |
""",
xml__xpath_match=[
xp.model_instance_choices("choices", (("1", "One"), ("2", "Two"))),
xp.body_select1_itemset("a"),
xpc.model_instance_choices_label("choices", (("1", "One"), ("2", "Two"))),
xpq.body_select1_itemset("a"),
],
)

Expand All @@ -39,8 +40,8 @@ def test_numeric_choice_names__for_dynamic_selects__allowed(self):
| | choices | 2 | Two |
""",
xml__xpath_match=[
xp.model_instance_choices("choices", (("1", "One"), ("2", "Two"))),
xp.body_select1_itemset("a"),
xpc.model_instance_choices_label("choices", (("1", "One"), ("2", "Two"))),
xpq.body_select1_itemset("a"),
],
)

Expand All @@ -59,8 +60,18 @@ def test_choices_without_labels__for_static_selects__allowed(self):
| | choices | 2 | |
""",
xml__xpath_match=[
xp.model_instance_choices_nl("choices", (("1", ""), ("2", ""))),
xp.body_select1_itemset("a"),
xpq.body_select1_itemset("a"),
"""
/h:html/h:head/x:model/x:instance[@id='choices']/x:root[
./x:item/x:name/text() = '1'
and not(./x:item/x:label)
and not(./x:item/x:itextId)
and
./x:item/x:name/text() = '2'
and not(./x:item/x:label)
and not(./x:item/x:itextId)
]
"""
],
)

Expand All @@ -80,7 +91,17 @@ def test_choices_without_labels__for_dynamic_selects__allowed_by_pyxform(self):
""",
run_odk_validate=False,
xml__xpath_match=[
xp.model_instance_choices_nl("choices", (("1", ""), ("2", ""))),
xp.body_select1_itemset("a"),
xpq.body_select1_itemset("a"),
"""
/h:html/h:head/x:model/x:instance[@id='choices']/x:root[
./x:item/x:name/text() = '1'
and not(./x:item/x:label)
and not(./x:item/x:itextId)
and
./x:item/x:name/text() = '2'
and not(./x:item/x:label)
and not(./x:item/x:itextId)
]
"""
],
)
19 changes: 12 additions & 7 deletions tests/test_dynamic_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
from time import perf_counter
from typing import Optional, Tuple
from unittest.mock import patch
from tests.xpath_helpers.choices import xp as xpc

import psutil

from pyxform import utils
from tests.pyxform_test_case import PyxformTestCase
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq


@dataclass()
Expand Down Expand Up @@ -577,12 +578,16 @@ def test_dynamic_default_select_choice_name_with_hyphen(self):
xp.model(1, Case(False, "select_one", "a-2")),
xp.model(2, Case(False, "select_one", "1-1")),
xp.model(3, Case(False, "select_one", "a-b")),
xpc.model_instance_choices("c1", (("a-1", "C A-1"), ("a-2", "C A-2"))),
xpc.model_instance_choices("c2", (("1-1", "C 1-1"), ("2-2", "C 1-2"))),
xpc.model_instance_choices("c3", (("a-b", "C A-B"),)),
xpc.body_select1_itemset("q1"),
xpc.body_select1_itemset("q2"),
xpc.body_select1_itemset("q3"),
xpc.model_instance_choices_label(
"c1", (("a-1", "C A-1"), ("a-2", "C A-2"))
),
xpc.model_instance_choices_label(
"c2", (("1-1", "C 1-1"), ("2-2", "C 1-2"))
),
xpc.model_instance_choices_label("c3", (("a-b", "C A-B"),)),
xpq.body_select1_itemset("q1"),
xpq.body_select1_itemset("q2"),
xpq.body_select1_itemset("q3"),
],
)

Expand Down
9 changes: 6 additions & 3 deletions tests/test_external_instances_for_selects.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
from tests.pyxform_test_case import PyxformTestCase
from tests.test_utils.md_table import md_table_to_workbook
from tests.utils import get_temp_dir
from tests.xpath_helpers.choices import xp as xpc
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq


@dataclass()
Expand Down Expand Up @@ -319,8 +320,10 @@ def test_no_params_with_filters(self):
]
""",
# select_one generates internal select.
xpc.model_instance_choices("state", (("nsw", "NSW"), ("vic", "VIC"))),
xpc.body_select1_itemset("state"),
xpc.model_instance_choices_label(
"state", (("nsw", "NSW"), ("vic", "VIC"))
),
xpq.body_select1_itemset("state"),
# select_one_external generates input referencing itemsets.csv
"""
/h:html/h:body[.
Expand Down
13 changes: 8 additions & 5 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
Test duplicate survey question field name.
"""
from tests.pyxform_test_case import PyxformTestCase
from tests.xpath_helpers.choices import xp as xpc
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq


class FieldsTests(PyxformTestCase):
Expand Down Expand Up @@ -118,8 +119,10 @@ def test_duplicate_choices_with_allow_choice_duplicates_setting(self):
self.assertPyxformXform(
md=md,
xml__xpath_match=[
xpc.model_instance_choices("list", (("a", "A"), ("b", "B"), ("b", "C"))),
xpc.body_select1_itemset("S1"),
xpc.model_instance_choices_label(
"list", (("a", "A"), ("b", "B"), ("b", "C"))
),
xpq.body_select1_itemset("S1"),
],
)

Expand All @@ -139,8 +142,8 @@ def test_choice_list_without_duplicates_is_successful(self):
self.assertPyxformXform(
md=md,
xml__xpath_match=[
xpc.model_instance_choices("list", (("a", "A"), ("b", "B"))),
xpc.body_select1_itemset("S1"),
xpc.model_instance_choices_label("list", (("a", "A"), ("b", "B"))),
xpq.body_select1_itemset("S1"),
],
)

Expand Down
49 changes: 17 additions & 32 deletions tests/test_rank.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
Test rank widget.
"""
from tests.pyxform_test_case import PyxformTestCase
from tests.xpath_helpers.choices import xp as xpc
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq


class RangeWidgetTest(PyxformTestCase):
Expand All @@ -19,8 +20,8 @@ def test_rank(self):
| | mylist | b | B |
""",
xml__xpath_match=[
xpc.model_instance_choices("mylist", (("a", "A"), ("b", "B"))),
xpc.body_odk_rank_itemset("order"), # also an implicit test for xmlns:odk
xpc.model_instance_choices_label("mylist", (("a", "A"), ("b", "B"))),
xpq.body_odk_rank_itemset("order"), # also an implicit test for xmlns:odk
"/h:html/h:head/x:model/x:bind[@nodeset='/test_name/order' and @type='odk:rank']",
],
)
Expand Down Expand Up @@ -56,6 +57,7 @@ def test_rank_filter(self):
)

def test_rank_translations(self):
"""Should find itext/translations for rank, using itemset method."""
self.assertPyxformXform(
md="""
| survey | | | | |
Expand All @@ -67,35 +69,18 @@ def test_rank_translations(self):
| | mylist | b | B | BB |
""",
xml__xpath_match=[
xpc.model_instance_choices("mylist", (("a", "A"), ("b", "B"))),
xpc.body_odk_rank_itemset("order"), # also an implicit test for xmlns:odk
xpc.model_instance_choices_itext("mylist", ("a", "b")),
xpq.body_odk_rank_itemset("order"), # also an implicit test for xmlns:odk
"/h:html/h:head/x:model/x:bind[@nodeset='/test_name/order' and @type='odk:rank']",
# All itemset translations.
xpc.model_itext_choice_text_label_by_pos("default", "mylist", ("A", "B")),
xpc.model_itext_choice_text_label_by_pos(
"French (fr)", "mylist", ("AA", "BB")
),
# No non-itemset translations.
xpc.model_itext_no_text_by_id("default", "/test_name/order/a:label"),
xpc.model_itext_no_text_by_id("default", "/test_name/order/b:label"),
xpc.model_itext_no_text_by_id("French (fr)", "/test_name/order/a:label"),
xpc.model_itext_no_text_by_id("French (fr)", "/test_name/order/b:label"),
],
# TODO: fix this test
debug=True,
# xml__contains=[
# 'xmlns:odk="http://www.opendatakit.org/xforms"',
# '<translation lang="French (fr)">',
# """<text id="/data/order:label">
# <value>Ranger</value>
# </text>""",
# """<text id="/data/order/a:label">
# <value>AA</value>
# </text>""",
# """<text id="/data/order/b:label">
# <value>BB</value>
# </text>""",
# "</translation>",
# """<odk:rank ref="/data/order">
# <label ref="jr:itext('/data/order:label')"/>
# <item>
# <label ref="jr:itext('/data/order/a:label')"/>
# <value>a</value>
# </item>
# <item>
# <label ref="jr:itext('/data/order/b:label')"/>
# <value>b</value>
# </item>
# </odk:rank>""",
# ],
)
Loading