Page MenuHomePhabricator

Send quantities as strings
Needs RevisionPublic

Authored by matej_suchanek on Dec 9 2017, 5:11 PM.


Patch without arc
git checkout -b D911 && curl -L | git apply

What happens:

What we want:

When I modified the POST request in the console and wrapped the number in quotes, it produced the second diff.

Diff Detail

Lint Skipped
Unit Tests Skipped

Event Timeline

matej_suchanek created this revision.

LGTM. I’ll do “accept revision” and hope that doesn’t merge the change yet, because I think that should be left to Magnus :)

This revision is now accepted and ready to land.Dec 9 2017, 6:24 PM
thiemowmde added a subscriber: thiemowmde.

A straight string conversion will not work in cases like 0.000000002. PHP will turn this into 2.0E-9, and the Wikibase deserializers will not accept this. See for more test cases.

This revision now requires changes to proceed.Dec 12 2017, 4:42 PM

Hm, so this is trivial for the quantity amount (and whole boundless value) but not for the bounds since I need to work with the floats.

What do you suggest? I have honestly no idea how to fix this (except for using the code from your patch). Floats are scary anyway...

As this is my first patch accept in Differential, how do I merge this here? No "merge" button that I can see on this page. Probably not "commandeer", whatever that is?

@thiemowmde will something simple like sprintf( '%.16f', $value ) work? (Importing the DataValues library would probably be the proper solution, but I’m not sure how easy this is, and it would be nice to get some version of this change merged soon.)

@Lucas_Werkmeister_WMDE Looks promising but also problematic:

> for ( $i = 1; $i < 20; ++$i ) {
  echo sprintf( "%2d: %.${i}F\n", $i, 34.708 );

 1: 34.7
 2: 34.71
 3: 34.708
 4: 34.7080
 5: 34.70800
 6: 34.708000
 7: 34.7080000
 8: 34.70800000
 9: 34.708000000
10: 34.7080000000
11: 34.70800000000
12: 34.708000000000
13: 34.7080000000000
14: 34.70800000000000
15: 34.707999999999998
16: 34.7079999999999984
17: 34.70799999999999841
18: 34.707999999999998408
19: 34.7079999999999984084

@matej_suchanek ah, I didn’t think of that – a float has approximately 16 digits of decimal precision (if I’m not mistaken), but for numbers outside of [0, 1] not all of this precision is spent on the decimal part, so if we ask for 15 digits behind the decimal point of 0.34708, that’s fine, but 15 digits behind the decimal point of 34.708 exceeds the total precision of the stored value.

Hang on. For the amount, can’t we just send $m[1]? That’s a string, right? Unfortunately it doesn’t solve the problem for the upper and lower bounds (where we somehow need to add $value and $error without losing precision), but unbounded values are probably the more common case anyways.

Yes, the trouble is just with bounded quantities.

Continuing in the brainstorming, what about [[ | number_format ]]?

Also brainstorming – one conceptually very simple solution would of course be to delegate to wbparsevalue, so QS would only parse the unit part. But that’s either very slow (lots of synchronous API calls) or rather slow and very complicated (you can parse up to 50 values in a single API call, but then you have to aggregate them in QS somehow).

In the meantime, I’ve uploaded D948 to fix just the part that we can easily fix.