Skip to content

Commit

Permalink
chg: access attributes directly instead of via o.get(k) or o[k] etc.
Browse files Browse the repository at this point in the history
- classes have attributes defined now so no need to use them like dicts
- provides better warnings / navigation / refactoring via IDE
- variable impact on performance (some slightly better, some worse)
- also converted a few more string concatenations to f-strings
  • Loading branch information
lindsay-stevens committed Dec 10, 2024
1 parent 7096cbf commit dfda327
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 60 deletions.
4 changes: 2 additions & 2 deletions pyxform/entities/entity_declaration.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def __init__(self, name: str, type: str, parameters: dict, **kwargs):
super().__init__(name=name, **kwargs)

def xml_instance(self, **kwargs):
parameters = self.get(const.PARAMETERS, {})
parameters = self.parameters

attributes = {
EC.DATASET.value: parameters.get(EC.DATASET, ""),
Expand Down Expand Up @@ -75,7 +75,7 @@ def xml_bindings(self, survey: "Survey"):
"""
See the class comment for an explanation of the logic for generating bindings.
"""
parameters = self.get(const.PARAMETERS, {})
parameters = self.parameters
entity_id_expression = parameters.get(EC.ENTITY_ID, None)
create_condition = parameters.get(EC.CREATE_IF, None)
update_condition = parameters.get(EC.UPDATE_IF, None)
Expand Down
69 changes: 29 additions & 40 deletions pyxform/question.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,27 +150,20 @@ def validate(self):
raise PyXFormError(f"Unknown question type '{self.type}'.")

def xml_instance(self, survey: "Survey", **kwargs):
attributes = self.get("instance")
attributes = self.instance
if attributes is None:
attributes = {}
else:
for key, value in attributes.items():
attributes[key] = survey.insert_xpaths(value, self)

if self.get("default") and not default_is_dynamic(self.default, self.type):
return node(self.name, str(self.get("default")), **attributes)
if self.default and not default_is_dynamic(self.default, self.type):
return node(self.name, str(self.default), **attributes)
return node(self.name, **attributes)

def xml_control(self, survey: "Survey"):
if self.type == "calculate" or (
(
(
hasattr(self, "bind")
and self.bind is not None
and "calculate" in self.bind
)
or self.trigger
)
(self.bind is not None and "calculate" in self.bind or self.trigger)
and not (self.label or self.hint)
):
nested_setvalues = survey.get_trigger_values_for_question_name(
Expand Down Expand Up @@ -269,13 +262,13 @@ def build_xml(self, survey: "Survey"):
result.appendChild(element)

# Input types are used for selects with external choices sheets.
if self["query"]:
choice_filter = self.get(constants.CHOICE_FILTER)
if self.query:
choice_filter = self.choice_filter
if choice_filter is not None:
pred = survey.insert_xpaths(choice_filter, self, True)
query = f"""instance('{self["query"]}')/root/item[{pred}]"""
query = f"""instance('{self.query}')/root/item[{pred}]"""
else:
query = f"""instance('{self["query"]}')/root/item"""
query = f"""instance('{self.query}')/root/item"""
result.setAttribute("query", query)
return result

Expand Down Expand Up @@ -427,42 +420,40 @@ def build_xml(self, survey: "Survey"):

# itemset are only supposed to be strings,
# check to prevent the rare dicts that show up
if self["itemset"] and isinstance(self["itemset"], str):
itemset, file_extension = os.path.splitext(self["itemset"])
if self.itemset and isinstance(self.itemset, str):
itemset, file_extension = os.path.splitext(self.itemset)

if file_extension == ".geojson":
itemset_value_ref = EXTERNAL_CHOICES_ITEMSET_REF_VALUE_GEOJSON
itemset_label_ref = EXTERNAL_CHOICES_ITEMSET_REF_LABEL_GEOJSON
else:
itemset_value_ref = EXTERNAL_CHOICES_ITEMSET_REF_VALUE
itemset_label_ref = EXTERNAL_CHOICES_ITEMSET_REF_LABEL
if hasattr(self, "parameters") and self.parameters is not None:
if self.parameters is not None:
itemset_value_ref = self.parameters.get("value", itemset_value_ref)
itemset_label_ref = self.parameters.get("label", itemset_label_ref)

multi_language = self.get("_itemset_multi_language", False)
has_media = self.get("_itemset_has_media", False)
has_dyn_label = self.get("_itemset_dyn_label", False)
is_previous_question = bool(
PYXFORM_REFERENCE_REGEX.search(self.get("itemset"))
)
multi_language = self._itemset_multi_language
has_media = self._itemset_has_media
has_dyn_label = self._itemset_dyn_label
is_previous_question = bool(PYXFORM_REFERENCE_REGEX.search(self.itemset))

if file_extension in EXTERNAL_INSTANCE_EXTENSIONS:
pass
elif not multi_language and not has_media and not has_dyn_label:
itemset = self["itemset"]
itemset = self.itemset
else:
itemset = self["itemset"]
itemset = self.itemset
itemset_label_ref = "jr:itext(itextId)"

choice_filter = self.get(constants.CHOICE_FILTER)
choice_filter = self.choice_filter
if choice_filter is not None:
choice_filter = survey.insert_xpaths(
choice_filter, self, True, is_previous_question
)
if is_previous_question:
path = (
survey.insert_xpaths(self["itemset"], self, reference_parent=True)
survey.insert_xpaths(self.itemset, self, reference_parent=True)
.strip()
.split("/")
)
Expand All @@ -471,7 +462,7 @@ def build_xml(self, survey: "Survey"):
itemset_label_ref = path[-1]
if choice_filter:
choice_filter = choice_filter.replace(
"current()/" + nodeset, "."
f"current()/{nodeset}", "."
).replace(nodeset, ".")
else:
# Choices must have a value. Filter out repeat instances without
Expand All @@ -484,21 +475,18 @@ def build_xml(self, survey: "Survey"):
if choice_filter:
nodeset += f"[{choice_filter}]"

if self["parameters"]:
params = self["parameters"]
if self.parameters:
params = self.parameters

if "randomize" in params and params["randomize"] == "true":
nodeset = "randomize(" + nodeset
nodeset = f"randomize({nodeset}"

if "seed" in params:
if params["seed"].startswith("${"):
nodeset = (
nodeset
+ ", "
+ survey.insert_xpaths(params["seed"], self).strip()
)
seed = survey.insert_xpaths(params["seed"], self).strip()
nodeset = f"{nodeset}, {seed}"
else:
nodeset = nodeset + ", " + params["seed"]
nodeset = f"""{nodeset}, {params["seed"]}"""

nodeset += ")"

Expand Down Expand Up @@ -622,8 +610,9 @@ def build_xml(self, survey: "Survey"):
for key, value in control_dict.items():
control_dict[key] = survey.insert_xpaths(value, self)
control_dict["ref"] = self.get_xpath()
params = self.get("parameters", {})
control_dict.update(params)
params = self.parameters
if params:
control_dict.update(params)
result = node(**control_dict)
if label_and_hint:
for element in self.xml_label_and_hint(survey=survey):
Expand Down
4 changes: 2 additions & 2 deletions pyxform/section.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,10 @@ def xml_control(self, survey: "Survey"):
else:
attributes = {}

if not self.get("flat"):
if not self.flat:
attributes["ref"] = self.get_xpath()

if "label" in self and self.label is not None and len(self["label"]) > 0:
if self.label:
children.append(self.xml_label(survey=survey))
for n in Section.xml_control(self, survey=survey):
children.append(n)
Expand Down
12 changes: 6 additions & 6 deletions pyxform/survey_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ def to_json(self):

def json_dump(self, path=""):
if not path:
path = self.name + ".json"
path = f"{self.name}.json"
print_pyobj_to_json(self.to_json_dict(), path)

def __eq__(self, y):
Expand All @@ -330,14 +330,14 @@ def __eq__(self, y):

def _translation_path(self, display_element: str) -> str:
"""Get an itextId based on the element XPath and display type."""
return self.get_xpath() + ":" + display_element
return f"{self.get_xpath()}:{display_element}"

def get_translations(self, default_language):
"""
Returns translations used by this element so they can be included in
the <itext> block. @see survey._setup_translations
"""
bind_dict = self.get("bind")
bind_dict = self.bind
if bind_dict and isinstance(bind_dict, dict):
constraint_msg = bind_dict.get("jr:constraintMsg")
if isinstance(constraint_msg, dict):
Expand Down Expand Up @@ -410,11 +410,11 @@ def get_translations(self, default_language):
display_element == "hint"
and not isinstance(label_or_hint, dict)
and hasattr(self, "hint")
and self.get("hint") is not None
and self.hint is not None
and len(label_or_hint) > 0
and hasattr(self, "guidance_hint")
and self.get("guidance_hint") is not None
and len(self["guidance_hint"]) > 0
and self.guidance_hint is not None
and len(self.guidance_hint) > 0
):
label_or_hint = {default_language: label_or_hint}

Expand Down
10 changes: 5 additions & 5 deletions tests/test_dynamic_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -778,11 +778,11 @@ def test_dynamic_default_performance__time(self):
Results with Python 3.10.14 on VM with 2vCPU (i7-7700HQ) 1GB RAM, x questions
each, average of 10 runs (seconds), with and without the check, per question:
| num | with | without | peak RSS MB |
| 500 | 0.2371 | 0.2459 | 58 |
| 1000 | 0.3819 | 0.3724 | 63 |
| 2000 | 0.7198 | 0.7539 | 67 |
| 5000 | 1.7854 | 1.8953 | 92 |
| 10000 | 3.8543 | 3.8675 | 128 |
| 500 | 0.1903 | 0.1977 | 58 |
| 1000 | 0.4010 | 0.3913 | 63 |
| 2000 | 0.6860 | 0.6813 | 67 |
| 5000 | 1.7119 | 1.7421 | 90 |
| 10000 | 3.5399 | 3.4963 | 136 |
"""
survey_header = """
| survey | | | | |
Expand Down
10 changes: 5 additions & 5 deletions tests/test_translations.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,11 @@ def test_missing_translations_check_performance(self):
with 2 choices each, average of 10 runs (seconds), with and without the check,
per question:
| num | with | without | peak RSS MB |
| 500 | 0.7427 | 0.7448 | 76 |
| 1000 | 1.7647 | 1.8032 | 97 |
| 2000 | 5.2996 | 5.0266 | 139 |
| 5000 | 22.862 | 21.334 | 253 |
| 10000 | 69.730 | 68.568 | 437 |
| 500 | 0.8251 | 0.8473 | 76 |
| 1000 | 1.8430 | 1.8612 | 97 |
| 2000 | 5.0824 | 5.1167 | 140 |
| 5000 | 19.921 | 21.390 | 249 |
| 10000 | 78.382 | 74.223 | 435 |
"""
survey_header = """
| survey | | | | |
Expand Down

0 comments on commit dfda327

Please sign in to comment.