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 all commits
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
2 changes: 1 addition & 1 deletion pyxform/aliases.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"select one": constants.SELECT_ONE,
"select_multiple": constants.SELECT_ALL_THAT_APPLY,
"select all that apply": constants.SELECT_ALL_THAT_APPLY,
"select_one_external": "select one external",
"select_one_external": constants.SELECT_ONE_EXTERNAL,
"select_one_from_file": constants.SELECT_ONE,
"select_multiple_from_file": constants.SELECT_ALL_THAT_APPLY,
"select one from file": constants.SELECT_ONE,
Expand Down
64 changes: 38 additions & 26 deletions pyxform/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
import copy
import os
import re
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
Expand All @@ -24,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):
"""
Expand Down Expand Up @@ -87,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)
Expand Down Expand Up @@ -142,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()

Expand All @@ -151,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 [
Expand All @@ -172,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):
Expand Down Expand Up @@ -219,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"],
Expand All @@ -241,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)

Expand Down Expand Up @@ -338,13 +354,12 @@ def create_survey_from_xls(path_or_file, default_name=None):


def create_survey(
name_of_main_section=None,
sections=None,
main_section=None,
id_string=None,
title=None,
default_language=None,
):
name_of_main_section: str = None,
sections: Dict[str, Dict] = None,
main_section: Dict[str, Any] = None,
id_string: Optional[str] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

id_string seems like it shouldn't be optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I sprinkled on this type annotation to help understand data flow. The id_string (and perhaps other params) could be mandatory, but then it seemed to make sense to refactor the body of the function to remove is None checks, etc., which could easily snowball into more refactoring.

title: Optional[str] = None,
) -> Survey:
"""
name_of_main_section -- a string key used to find the main section in the
sections dict if it is not supplied in the
Expand Down Expand Up @@ -380,11 +395,10 @@ def create_survey(

if title is not None:
survey.title = title
survey.def_lang = default_language
return survey


def create_survey_from_path(path, include_directory=False):
def create_survey_from_path(path: str, include_directory: bool = False) -> Survey:
"""
include_directory -- Switch to indicate that all the survey forms in the
same directory as the specified file should be read
Expand All @@ -398,6 +412,4 @@ def create_survey_from_path(path, include_directory=False):
else:
main_section_name, section = file_utils.load_file_to_dict(path)
sections = {main_section_name: section}
pkg = {"name_of_main_section": main_section_name, "sections": sections}

return create_survey(**pkg)
return create_survey(name_of_main_section=main_section_name, sections=sections)
7 changes: 7 additions & 0 deletions pyxform/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
MEDIA = "media"
CONTROL = "control"
APPEARANCE = "appearance"
ITEMSET = "itemset"
RANDOMIZE = "randomize"
CHOICE_FILTER = "choice_filter"
PARAMETERS = "parameters"

LOOP = "loop"
COLUMNS = "columns"
Expand All @@ -56,7 +60,9 @@
CHILDREN = "children"

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"

Expand All @@ -65,6 +71,7 @@
CASCADING_SELECT = "cascading_select"
TABLE_LIST = "table-list" # hyphenated because it goes in appearance, and convention for appearance column is dashes # noqa
FIELD_LIST = "field-list"
LIST_NOLABEL = "list-nolabel"

SURVEY = "survey"
SETTINGS = "settings"
Expand Down
18 changes: 15 additions & 3 deletions pyxform/question.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
XForm Survey element classes for different question types.
"""
import os.path
import re

from pyxform.constants import (
EXTERNAL_CHOICES_ITEMSET_REF_LABEL,
Expand All @@ -15,7 +14,12 @@
from pyxform.errors import PyXFormError
from pyxform.question_type_dictionary import QUESTION_TYPE_DICT
from pyxform.survey_element import SurveyElement
from pyxform.utils import default_is_dynamic, has_dynamic_label, node
from pyxform.utils import (
PYXFORM_REFERENCE_REGEX,
default_is_dynamic,
has_dynamic_label,
node,
)


class Question(SurveyElement):
Expand Down Expand Up @@ -155,6 +159,9 @@ def xml(self):
def validate(self):
pass

def xml_control(self):
raise NotImplementedError()


class MultipleChoiceQuestion(Question):
def __init__(self, **kwargs):
Expand Down Expand Up @@ -222,7 +229,9 @@ def build_xml(self):

has_media = False
has_dyn_label = False
is_previous_question = bool(re.match(r"^\${.*}$", self.get("itemset")))
is_previous_question = bool(
PYXFORM_REFERENCE_REGEX.search(self.get("itemset"))
)

if choices.get(itemset):
has_media = bool(choices[itemset][0].get("media"))
Expand Down Expand Up @@ -326,6 +335,9 @@ def xml(self):
def validate(self):
pass

def xml_control(self):
raise NotImplementedError()


class OsmUploadQuestion(UploadQuestion):
def __init__(self, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion pyxform/section.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""
from pyxform.errors import PyXFormError
from pyxform.external_instance import ExternalInstance
from pyxform.question import SurveyElement
from pyxform.survey_element import SurveyElement
from pyxform.utils import node


Expand Down
Loading