XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
82 stars 136 forks source link

Add non-string calculation types #70

Closed yanokwa closed 4 years ago

yanokwa commented 8 years ago

EDIT: see https://github.com/XLSForm/pyxform/issues/70#issuecomment-599660252 for the latest proposed approach.

Initial bug reported at https://groups.google.com/d/msg/opendatakit/eYFsVCLet1U/Wb4wZumGBAAJ

"The problem seems to be that the total_feld_calc field is stored as a "string" field, so the subsequent sum(${total_field_calc}) will fail with a type conversion failure."

MartijnR commented 8 years ago

also discussed here: https://github.com/XLSForm/xlsform.github.io/issues/71

The bug is in javarosa unfortunately. There should be no problem storing the result as a string. An XPath evaluator has no knowledge of the datatype of expression function arguments or operands.

mitchellsundt commented 8 years ago

Adding note: this does impact downstream processing. I.e., when we publish to Google Sheets or Fusion Tables, we use the field type in the XForm bind to configure the downstream server data type and storage format.

So while I agree the JR expression evaluator should silently caste to the correct data type, this is still a spec issue within XLSForm, as it prevents publishing appropriately-typed fields in external datasets.

MartijnR commented 8 years ago

Agreed. For that use case, it's a missing feature. Types: calculate_decimal, calculate_integer etc?

(I will just keep hoping the JavaRosa type casting bugs will get fixed).

mitchellsundt commented 8 years ago

Or if it can be like the select prompt types:

calculate calculate integer calculate decimal calculate geopoint

i.e., with the optional second term being the type of the field. Defaulting to string.

MartijnR commented 8 years ago

Yes, looks good to me.

P.S. This will mean Enketo & ODK Aggregate users will start running into HTTP 500 responses from ODK Aggregate for this Aggregate bug when the integer or decimal calculation returns NaN, Infinity or -Infinity.

MartijnR commented 6 years ago

Just for fun here is a hacky way to do this currently with bind::type: https://docs.google.com/spreadsheets/d/1zpb3PyN6sVIH06CJtqfPVB9UlF8_OQDdSky-M5r5Lo0/edit#gid=0

MartijnR commented 5 years ago

another use case here: https://community.kobotoolbox.org/t/link-answer-to-geopoint/2446/7

TL/DR;

Useful to geo datatype calculations so that servers (such as KoBo) know the data can be displayed on a map.

matthew-white commented 4 years ago

Another related issue was reported on the ODK forum here. To summarize: a calculate field will be string in the ODK Central OData feed even if the field is always numeric.

MartijnR commented 4 years ago

During discussion for #438, we agreed to implement this like this:

type name label hint calculation
dateTime c now()

output:

<bind nodeset="/data/c" type="dateTime" calculate="now()" />

The rule would become: If a question has a calculation but it has neither label nor hint value, it will not get a body element (just a <bind>).

This new rule builds upon the existing rule that a question without a calculation needs to have either a label or a hint to pass pyxform validation.

This means that these two questions below would become equivalent (because "string" is the default type), and essentially type=calculate has become superfluous (not proposing to remove this though):

type name label hint calculation
calculate a 'a'
text b 'a'
lognaturel commented 4 years ago

The approach described at https://github.com/XLSForm/pyxform/issues/70#issuecomment-599660252 is now in master. It needs to be documented before release.