MobilityData / gbfs

Documentation for the General Bikeshare Feed Specification, a standardized data feed for shared mobility system availability. Maintained by MobilityData
https://gbfs.org
Other
784 stars 286 forks source link

Price Type #116

Closed PierrickP closed 4 years ago

PierrickP commented 5 years ago

Following #113, i check all urls on systems.csv

On docs, it's

This should be in the base unit as defined by the ISO 4217 currency code with the appropriate number of decimal places and omitting the currency symbol. e.g.

Can we clarify the JSON type ? Number and/or String Most looks number but some are given in String https://rey.publicbikesystem.net/ube/gbfs/v1/en/system_pricing_plans (and with ',')

maduprasPBSC commented 5 years ago

Thanks for pointing out the issue with Reykjavik currency. We are going to apply a change to standardize the currency format for all our cities using the ###.## format. But to clarify, is the currency field supposed to be a string or a double? It's unclear from the documentation.

PierrickP commented 5 years ago

Oh at first i didn't notice it was in ISK. The "," for the thousands is a commom delimiter.

They are many ISO available for defined amount in currency.

I vote for the 3rd.

antrim commented 5 years ago

(Added text to the title title to indicate the field definitions being discussed.)

@maduprasPBSC, @PierrickP : This looks like a useful clarifying change! But would there be any reason NOT to define the "price" field as a number? i.e. Are there any cases where a GBFS consumer need to parse "price" as a string?

PierrickP commented 5 years ago

@antrim still voting for W3C's webpayment. They already braistorming on it ;) Amount should be https://www.w3.org/TR/payment-request/#dfn-valid-decimal-monetary-value

jcn commented 5 years ago

From reading the spec, webpayment looks like the value is explicitly a string, but it also explicitly a decimal monetary value.

I would be open to specifying price as a string, if only to ensure that precision is never lost in transit.

heidiguenin commented 4 years ago

If producers are currently using both string & number, then it sounds like, for now, we should at least clarify in the spec that both are allowed to maintain backwards compatibility.

But for the future, is it worth considering a breaking change to narrow the definition to only allow what @PierrickP suggests - a string that represents a valid decimal monetary value? If so, let's discuss that so we can tee up a PR for v2.

heidiguenin commented 4 years ago

The spec's been updated to explicitly allow both string and non-negative float, and I've just opened PR #216 to limit the price type to string in the future. I'm going to close this issue in the next couple days, and we can move the conversation to the PR.