vimeo / py-money

Money class for Python 3
MIT License
125 stars 27 forks source link

This updates some of the sub unit values #12

Open matthewsnell opened 4 years ago

matthewsnell commented 4 years ago

This fixes the issue #9 Values were taken from https://developers.facebook.com/docs/marketing-api/currencies/

nickyr commented 4 years ago

I'll definitely release this with a new major version

matthewsnell commented 4 years ago

I would probably lean more towards using Facebook's values as well.

nickyr commented 4 years ago

@shridharama @jcmanzo @kaylafuchs @DanayaMel @brendanpeters @lorenzocastillo I will merge this on Wednesday if nobody objects.

kaylafuchs commented 4 years ago

No objections here! I would be inclined to trust Facebook API docs over Wikipedia but it would be nice to have a more authoritative source for this kind of thing. 🤔

Edit: after looking more closely at those Facebook docs it seems that they're tweaking some currency rules in order to set the minimum allowed bid on their ad units. In that case we may not want to trust them.

jcmanzo commented 4 years ago

Looks like this never got merged. I'll take care of it later today..

kaylafuchs commented 4 years ago

@jcmanzo I think there's still an open question here about trusting the Facebook docs for Colombia and Costa Rica because they're using sub units to set a minimum allowed bid on their ad units - https://developers.facebook.com/docs/marketing-api/currencies/#example-offset-1

suspectpart commented 3 years ago

I guess the canonical source of truth here should be ISO 4217 which delineates currency designators, country codes and sub units. Wikipedia states:

ISO 4217 is a standard first published by International Organization for Standardization in 1978, which delineates currency designators, country codes (alpha and numeric), and references to minor units in three tables [...]. The ISO 4217 code list is used in banking and business globally. In many countries the ISO codes for the more common currencies are so well known publicly that exchange rates published in newspapers or posted in banks use only these to delineate the currencies, instead of translated currency names or ambiguous currency symbols. ISO 4217 codes are used on airline tickets and international train tickets to remove any ambiguity about the price.

Even when you encounter sub units rarely in some countries (e.g. for the Forint in Hungary), we should still stick to the Standard if it nominally allows sub units.

The latest version of the Standard was published on 29 August, 2018 and can be found here as an XML or XLS file.

I ran a small script that loads the XLS file, runs every currency through the CurrencyHelper and compares the sub units to the ones specified in the CurrencyHelper. These are the results:

ISO_4217 (Published 2018-08-29)
----------------
Can't process ANTARCTICA (No universal currency)
Can't process PALESTINE, STATE OF (No universal currency)
Can't process SOUTH GEORGIA AND THE SOUTH SANDWICH ISLANDS (No universal currency)
[BIF] expected 1, got 100
[CLF] expected 10000, got 100
[CLP] expected 1, got 100
[DJF] expected 1, got 100
[GNF] expected 1, got 100
[ISK] expected 1, got 100
[KMF] expected 1, got 100
[KRW] expected 1, got 100
[MGA] expected 100, got 5
[MRU] Currency Missing! (expected 100)
[PYG] expected 1, got 100
[RWF] expected 1, got 100
[STN] Currency Missing! (expected 100)
[UGX] expected 1, got 100
[UYI] expected 1, got 100
[UYW] Currency Missing! (expected 10000)
[VES] Currency Missing! (expected 100)
[VND] expected 1, got 10
[XAF] expected 1, got 100
[XAG] expected 'N.A.', got 100
[XAU] expected 'N.A.', got 100
[XBA] expected 'N.A.', got 100
[XBB] expected 'N.A.', got 100
[XBC] expected 'N.A.', got 100
[XBD] expected 'N.A.', got 100
[XDR] expected 'N.A.', got 100
[XOF] expected 1, got 100
[XPD] expected 'N.A.', got 100
[XPF] expected 1, got 100
[XPT] expected 'N.A.', got 100
[XSU] expected 'N.A.', got 100
[XTS] expected 'N.A.', got 100
[XUA] expected 'N.A.', got 100
[XXX] Currency Missing! (expected N.A.)

I would suggest to approach this as follows:

  1. Discuss whether whether 100 is the correct sub units value for all X currencies (those are not real national currencies, but rather a mixture of supranational currencies and procedural purposes)
  2. Provide the missing currencies
  3. Fix all currencies where the sub units differ from the ones stated in ISO 4217. That would be:
    • [BIF] expected 1, got 100
    • [CLF] expected 10000, got 100
    • [CLP] expected 1, got 100
    • [DJF] expected 1, got 100
    • [GNF] expected 1, got 100
    • [ISK] expected 1, got 100
    • [KMF] expected 1, got 100
    • [KRW] expected 1, got 100
    • [PYG] expected 1, got 100
    • [RWF] expected 1, got 100
    • [UGX] expected 1, got 100
    • [UYI] expected 1, got 100
    • [VND] expected 1, got 10

If we implement these changes, we could add a remark to the README.md stating that py-money complies with ISO 4217, making it suitable for finance and business applications that need to comply with this Standard.

We could create an integration test that runs the ISO 4217 file through the CurrencyHelper to assert compliance. If it is ever going to change in the future, you just drop the new specification and get instant feedback about necessary adjustments.