Pass through itemset ref case

lognaturel commented 1 year ago

Closes #642

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

I decided to go with an exclude list for case normalization when first parsing parameters. The alternative I considered was to move case normalization to when parameter values are used by pyxform. I don't feel strongly between the two. Currently label and value are the only parameters that get set to user-provided strings. All of the others are numeric or enum values.

I also considered making sure all clients use label and value case-insensitively and leaving pyxform as it is. Even if we do choose to go that route, I don't think pyxform should be responsible for case normalization.

What are the regression risks?

The only risk I can see would be if some client somewhere counts on label and value references always being lowercase. However, alternate clients are not common.

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

No.

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

lognaturel commented 1 year ago

@lindsay-stevens although this has been an issue for a while, now that we know about it I think it should be fixed as soon as we can. There's a Central release that's planned for next week and I'd love to include it.

Very interested in seeing what you think of the approach and whether you find it pragmatic or backwards.

tiritea commented 1 year ago

Not sure I entirely understand the full context, but...

The only risk I can see would be if some client somewhere counts on label and value references always being lowercase.

In my client I'm certainly not making assumptions that select values are supposed to be case-insensitive (eg always lowercase).

FWIW I dont see any mention of case (in)sensitivity in the spec: https://www.w3.org/TR/2003/REC-xforms-20031014/slice8.html#ui-common-choices-value

lognaturel commented 1 year ago

select values

This is about the value and label references for defining itemsets. By default the value reference is name and the label reference is label. But those can be customized if e.g. you are using a file from another system that doesn't nave name and label elements/attributes/columns. In your XLSForm, you can specify something like value=XYZ_ID label=DisplayName. But currently the two values (XYZ_ID and DisplayName) are set to lowercase.

In an XML context it would be very unusual/go against the spec to treat element names as case-insensitive. I think it's would be a bit weird with CSV column names and geojson properties too though probably more common. Regardless, it's clear to me that pyxform should pass through the case provided by the user.

tiritea commented 1 year ago

Regardless, it's clear to me that pyxform should pass through the case provided by the user.

+1

XLSForm / pyxform