Add initial support for entities

[lognaturel]

Closes #622

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

This adds the desired support without increasing the size of workbook_to_json. It keeps the entities functionality relatively isolated which aligns well with the idea that this is an optional, experimental spec addition.

The entity node in the main instance has several attributes that each have bindings. I kept the JSON representation simple by using the existing parameters dictionary for the various aspects of entities that can be configured. Then I introduced a EntityDeclaration SurveyElement to create the attributes and their bindings. That feels better to me than trying to get an XForms-like representation in the json.

What are the regression risks?

I changed Survey to get the multiple bindings for a single form element. There used to be a xml_binding method for the current survey element's binding and xml_bindings for the bindings of the current element and descendants. I now have xml_bindings for the current survey element's possibly multiple bindings and xml_descendent_bindings for the bindings of the current element and descendants. I don't think there are regression risks but there's a chance this is being used as a public API. @ukanga @jnm any issues from your ends?

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

XLSForm/xlsform.github.io#232

Before submitting this PR, please make sure you have:

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

[ukanga]

I changed Survey to get the multiple bindings for a single form element. There used to be a xml_binding method for the current survey element's binding and xml_bindings for the bindings of the current element and descendants. I now have xml_bindings for the current survey element's possibly multiple bindings and xml_descendent_bindings for the bindings of the current element and descendants. I don't think there are regression risks but there's a chance this is being used as a public API. @ukanga @jnm any issues from your ends?

From a quick check, we are not using it directly as a public API. More checks are needed on my end but nothing should hold this work back from our end.

[lognaturel]

Thanks, @ukanga! If you find there’s an issue, we can make changes, even if it’s after an initial release.

[alxndrsn]

Is it important that this specific error is displayed when the label column is missing?

[lognaturel]

This error is because there are multiple entities declared. That gets checked before the label. https://github.com/XLSForm/pyxform/pull/624/files#diff-757cf40dfe1058a66a7d6603bb35a2ea70614e7df312df390058c21c4e9c0c62R206 tests that labels are required.

[yanokwa]

LGTM

[jnm]

Thanks for the mention, @lognaturel. I don't foresee any problems.

[lindsay-stevens]

@lognaturel I know this is merged and released but just checking if you still want me to review this per your slack message?

[lognaturel]

Thanks, @lindsay-stevens! There are two things that I think would be helpful to get your feedback on:

• structure of the JSON representation. https://github.com/XLSForm/pyxform/pull/624/files#diff-12546f84ff0c4f39841d2227089bad7f209656cdbf11441e8f0cba99f30091e3R49. I introduced a new entity type that the next layers can look for. I put everything else as parameters. For most other form components, the JSON structure matches the XForm structure. I didn't see any particular need to do that here and it would be awkward given the use of attributes. Interested in another opinion, though.
• thoughts on the entities package I introduced. It's new to have a package that represents a domain slice. I think that's appropriate here since the entities spec is this extra layer. It might be a pattern we can use more. I didn't try to establish any reusable infrastructure here and just put entity-related functions in entities_parsing.py. I think it's nice and simple and again maybe a first step we could take to breaking down workbook_to_json

High-level, I really tried not to make things messier and I want to confirm I achieved that. I'm also interested in whether you see any patterns we could/should reuse and/or any missed opportunities to lay useful groundwork.

[lindsay-stevens]

@lognaturel looks good to me. Some suggestions/notes

• representation:
  • probably best to stick with dicts since everything else is (unless/until it can be changed at once)
  • get_entity_declaration type hint could be more specific e.g. Dict[str, Union[str, Dict[str, str]]]
  • the entities dict placement in meta>children is pretty much where it goes in XML, seems good
• structure:
  • might be hard to tease out domain concepts, but maybe putting modules defining SurveyElement
    descendants together in an "survey_elements" package could be useful to separate from IO and other top-level modules.
  • A lot of workbook_to_json is validation which is why I started putting additions as modules in validation/pyxform. So I suggest moving "xlsparseutils.py" to "validators/xml_validations.py". Most of entities_parsing is validation too.
• general
  • entities_parsing pattern "if isinstance(x, bytes): x = x.encode('utf-8')" probably unnecessary in python3. If it is necessary then there should be a test (the tests pass with these removed).
  • Show error when there are save_tos in a repeat, simplify entities tests #636 the simple xlsform in test_dataset_in_entities_sheet__adds_meta_entity_block is used 7 times, I probably would put them in a separate TestCase, or stack all assertions together in one and comment them.
  • the linter PR check didn't catch that isort wants to fix xlsparseutils, I'll put a fix for that in another PR