Skip to content
This repository has been archived by the owner on Feb 19, 2022. It is now read-only.

Decimal support? #138

Closed
lhl opened this issue Dec 23, 2017 · 7 comments
Closed

Decimal support? #138

lhl opened this issue Dec 23, 2017 · 7 comments

Comments

@lhl
Copy link

lhl commented Dec 23, 2017

I was looking at various API libs and noticed that bitex doesn't seem to use decimal.Decimal() anywhere. How is math done accurately or is that something that's being punted on completely? (eg a quick search shows that balances simply get returned as string key/values?)

@MatthewCochrane
Copy link

Is decimal support necessary? There's not really any math that's done in this library at all at the moment. It just makes requests and (at the moment) returns the raw response. So most of the time the results are returned as strings
For example:

# Example response json
{'bid': '18629.0',
 'timestamp': '1513589744.5260189',
 'ask': '18630.0',
 'mid': '18629.5',
 'low': '18010'}

@bebosudo
Copy link

@lhl: we are discussing it right now in the implementation of the new formatter method, see #129.
The json response stores values as strings, but the formatter we're implementing should convert them to decimal, as you said, in order to avoid floating point inaccuracies.
Feel free to join the discussion!

@MatthewCochrane
Copy link

@bebosudo I tried to implement Decimal support and ran into a lot of problems. The major one was that sometimes the json responses are already converted to a float! For example when you do

# r is a request.Response
result = r.json()

result may have fields that are already converted to a float!! So when you convert them to a decimal:

# eg. result = {'bid': 0.001} Note that 'bid' value is not a string!
decimal_bid = Decimal(result['bid'])
print(decimal_bid)
# 0.0009999999999999999999999999993

To fix this you would need to replace the json method on requests.Response with one that parses floats to Decimals.
I think that would be difficult so for now I think we should stick with floats and not use Decimals at all.

The benefits of Decimal is limited anyway, who cares if you're out by one in 10^17. See examples below

from decimal import Decimal

print(Decimal('0.01'))
print(Decimal(0.01))

print(Decimal(0.01)/Decimal('0.01')-1) # error
print((Decimal(0.01)/Decimal('0.01')-1)*100) # error as %

f = float(0)
for i in range(10000000): # 10 million operations
    f += 0.01

true_result = Decimal('10000000')*Decimal('0.01')
float_result = Decimal(f)
error = (true_result-float_result)/true_result

print(true_result)
print(float_result)
print(error)

# Output
# 0.01
# 0.01000000000000000020816681711721685132943093776702880859375
# 2.0816681712E-17
# 2.081668171200E-15
# 100000.00
# 99999.999986309689120389521121978759765625
# 1.369031087961047887802124023E-10

Even if you do 10 million additions, the error on floats is still 1 in 1e10. That's 1 tenth of a billionth.

@bebosudo
Copy link

The major one was that sometimes the json responses are already converted to a float! For example when you do

# r is a request.Response
result = r.json()

result may have fields that are already converted to a float!!

The solution is passing the requests json() method the appropriate parsers to apply:

from decimal import Decimal
...
response.json(parse_int=Decimal, parse_float=Decimal)

(BTW, which exchange does send data as float numbers?)


The benefits of Decimal is limited anyway, who cares if you're out by one in 10^17.

Well, if I'm dealing with my money, I care of it.

>>> sum = 0.
>>> for _ in range(10):
...   sum += 0.1
... 
>>> sum
0.9999999999999999
>>> 1.1 + 2.2
3.3000000000000003

You don't need to go to millions to understand why floats are bad. Even if we produce an error in the order of 10^(-17) when dealing with something that can be really small such as cryptocurrencies, the overall result can be catastrophic for an end user using this library.

@deepbrook
Copy link
Collaborator

Using decimal for the formatted response is, IMO unnecessary - not because the use case isn't there; it's true that using the decimal library is better when handling money values - but I don't think see it being part of our job to supply that format.
A str representation is safe, and can be parsed by any library you may prefer to work with. Bitex is only meant to return data, but not do any calculations.

@MatthewCochrane
Copy link

MatthewCochrane commented Dec 27, 2017

@bebosudo Ok, a fair point. I still don't think the error from floats is a huge problem unless you're doing accounting and keeping track of peoples real money. I agree that decimal would be better, though I think there are some complications that would need to be resolved before we could use it.

Take a look at this ticker response from Bittrex:

{'success': True, 
 'result': [{'Ask': 0.01664998, 
             'BaseVolume': 2772.17956679, 
             'MarketName': 'BTC-LTC', 
             'OpenSellOrders': 9647, 
             'Low': 0.01589002, 
             'Created': '2014-02-13T00:00:00', 
             'OpenBuyOrders': 7846, 
             'Bid': 0.01663114, 
             'PrevDay': 0.01613444, 
             'High': 0.0171071, 
             'Volume': 168102.39042146, 
             'TimeStamp': '2017-12-18T09:42:47.75', 
             'Last': 0.01663114}], 
 'message': ''}

Other exchanges that do this are: CCEX, CoinCheck, Cryptopia, TheRockTrading.

This is a potential problem with some of the new formatting code we're adding to the library at the moment. When we call response.json() in the formatter we should probably be calling it like this

response.json(parse_float=str, parse_int=str)

or like this

from decimal import Decimal
response.json(parse_float=Decimal, parse_int=Decimal)

otherwise we're returning the results converted to floats, which are ever so slightly wrong.
As per @nlsdfnbch 's suggestion to stick with strings I suppose we should prefer the first approach.

It could be convenient to switch to Decimal at some point in the future depending on what the library is used for. It would save users having to pass formatted results through an extra function before they could use it in calculations.

@bebosudo
Copy link

bebosudo commented Dec 27, 2017

IMHO the best thing would be to return a decimal object in the formatted response we're creating and discussing in #129, and that's it.
Meanwhile the best option is to manually parse the json using the decimal builtin class, as we both demonstrated.

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

No branches or pull requests

4 participants