Why is this the best possible solution? Were any other approaches considered?
Seems like main reason for sorting in the first place was to help satisfy string-based matching for XML output tests. Since there are XPath test facilities now, it's not necessary. These changes remove explicit sorting in survey.py and utils.py, and remove order randomisation in xls2json.py that happened via conversion of keys to set objects - it now uses dicts instead so that order is preserved.
There is still a bit of sorting going on in pyxform_test_case.py and xform_test_case.py but since that's purely for testing it would not affect #602.
What are the regression risks?
If XForm consumers (such as Collect or Enketo or other tools) expected the sorted behaviour, those consumers would have to sort elements instead of expecting the form elements to be pre-sorted. Similarly, pyxform library users would need to apply a sort if needed e.g. in the survey choices dict.
It's possible that some tests might randomly fail either in CI or later on, because of the string matching thing. In which case it would only be a failure related to element or attribute order. I ran the test suite about a dozen times to try and catch them all.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No I would say this is an internal implementation detail. The spec doesn't say the elements should be sorted.
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
Closes #602
Why is this the best possible solution? Were any other approaches considered?
Seems like main reason for sorting in the first place was to help satisfy string-based matching for XML output tests. Since there are XPath test facilities now, it's not necessary. These changes remove explicit sorting in
survey.pyandutils.py, and remove order randomisation inxls2json.pythat happened via conversion of keys to set objects - it now uses dicts instead so that order is preserved.There is still a bit of sorting going on in
pyxform_test_case.pyandxform_test_case.pybut since that's purely for testing it would not affect #602.What are the regression risks?
If XForm consumers (such as Collect or Enketo or other tools) expected the sorted behaviour, those consumers would have to sort elements instead of expecting the form elements to be pre-sorted. Similarly, pyxform library users would need to apply a sort if needed e.g. in the survey choices dict.
It's possible that some tests might randomly fail either in CI or later on, because of the string matching thing. In which case it would only be a failure related to element or attribute order. I ran the test suite about a dozen times to try and catch them all.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No I would say this is an internal implementation detail. The spec doesn't say the elements should be sorted.
Before submitting this PR, please make sure you have:
testsnosetestsand verified all tests passblack pyxform teststo format code