Add support for geojson external files

[lognaturel]

Closes #591

Why is this the best possible solution? Were any other approaches considered?

Just adding some more conditions! It didn't feel like there were a ton of decision to make.

What are the regression risks?

Regression risk is isolated to external select types. The test coverage there is very high so it's hard to imagine what could go wrong.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

https://github.com/XLSForm/xlsform.github.io/issues/224

Before submitting this PR, please make sure you have:

• [x] included test cases for core behavior and edge cases in tests
• [x] run nosetests and verified all tests pass
• [x] run black pyxform tests to format code
• [x] verified that any code or assets from external sources are properly credited in comments

[lindsay-stevens]

Looks good! I suggest a minor change to use data so the rule is in one place, instead of repeating the conditional. Though it will work either way.

For constants.py

EXTERNAL_CHOICES_ITEMSET_REF_LABEL = {".xml": "label", ".csv": "label", ".geojson": "title"}
EXTERNAL_CHOICES_ITEMSET_REF_VALUE = {".xml": "name", ".csv": "name", ".geojson": "id"}

then in question.py

itemset_label_ref = self.parameters.get(
    "label", 
    EXTERNAL_CHOICES_ITEMSET_REF_VALUE[file_extension]
)
itemset_label_ref = self.parameters.get(
    "label", 
    EXTERNAL_CHOICES_ITEMSET_REF_LABEL[file_extension]
)

and in test_external_instances_for_selects.py

for ext, xp_city, xp_subs in self.xp_test_args:
    expected_value_ref = EXTERNAL_CHOICES_ITEMSET_REF_VALUE[ext]
    expected_label_ref = EXTERNAL_CHOICES_ITEMSET_REF_LABEL[ext]

[lognaturel]

Aha! I'm liking the general idea but the no file extension case makes it a bit messy. Either ''->name/''->label has to be in the dict or there still needs to be a conditional in question.py, right?

I do want to leave the tests with the explicit text we're looking for so it's not circular. That is, if both the code and the test refer to the same collection, we can't identify that the two have drifted apart.

[lindsay-stevens]

The empty string file extensions seem occur for regular choices e.g. select_one mylist. It begs the question as to why the Question subclasses don't have the original question type, or some indication that the question is a ..from_file select, rather than needing to split the itemset name to detect them. The following would handle the empty string case without the additional dict item:

itemset_value_ref = self.parameters.get(
    "value", 
    EXTERNAL_CHOICES_ITEMSET_REF_VALUE.get(file_extension, "name")
)
itemset_label_ref = self.parameters.get(
    "label", 
    EXTERNAL_CHOICES_ITEMSET_REF_LABEL.get(file_extension, "label")
)

or in SQL style

def coalesce(*args):
    return next((a for a in args if a is not None), None)

itemset_value_ref = coalesce(
    self.parameters.get("value"),
    EXTERNAL_CHOICES_ITEMSET_REF_VALUE.get(file_extension)
    "name"
)

593 would likely touch this code again so whatever we end up with might change again soon. So if the above isn't quite right either, I think we should merge without further changes.

[lognaturel]

why the Question subclasses don't have the original question type

Right, I did wonder that briefly. Let's leave it as-is for now and follow up next time we're in this area. I could imagine some tidying up as part of #590, too.