Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-string calculation types #70

Closed
yanokwa opened this issue Jun 13, 2016 · 10 comments · Fixed by #447
Closed

Add non-string calculation types #70

yanokwa opened this issue Jun 13, 2016 · 10 comments · Fixed by #447

Comments

@yanokwa
Copy link
Contributor

yanokwa commented Jun 13, 2016

EDIT: see #70 (comment) 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
Copy link
Contributor

MartijnR commented Jun 13, 2016

also discussed here: XLSForm/xlsform.github.io#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
Copy link
Contributor

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
Copy link
Contributor

MartijnR commented Jul 8, 2016

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).

@MartijnR MartijnR changed the title Calcs are stored as strings which can cause sums to fail Add non-string calculation types Jul 8, 2016
@mitchellsundt
Copy link
Contributor

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
Copy link
Contributor

MartijnR commented Aug 30, 2016

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
Copy link
Contributor

MartijnR commented Mar 28, 2018

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
Copy link
Contributor

MartijnR commented Feb 26, 2019

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
Copy link

matthew-white commented Dec 5, 2019

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
Copy link
Contributor

MartijnR commented Mar 16, 2020

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
Copy link
Contributor

The approach described at #70 (comment) is now in master. It needs to be documented before release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants