python-bittrex icon indicating copy to clipboard operation
python-bittrex copied to clipboard

Fix: Let JSON parser turn float-like values into Decimal

Open Tobiaqs opened this issue 7 years ago • 4 comments

Fix for #108

Tobiaqs avatar Dec 26 '17 11:12 Tobiaqs

So, if in not mistaken, every program that uses this library would have to change everything to decimal. Other exchange client libraries use floats. Do some avoid floats by using strings exclusively?

I think this change would break a lot of people.

unsupported operand type(s) for +: 'Decimal' and 'float'

skyl avatar Dec 26 '17 17:12 skyl

I think we can support Decimal usage but as @skyl says we must be smart about backwards compatibility. A simple idea would be to add this handler as a argument to the constructor

def using_requests(request_url, apisign, json_parse_args):
    return requests.get(
        request_url,
        headers={"apisign": apisign}
    ).json(**json_parse_args)

class Bittrex(object):
    def __init__(self, api_key, api_secret, calls_per_second=1, dispatch=using_requests, api_version=API_V1_1, json_parse=None):
        self.json_parse = json_parse if json_parse is not None else dict()
        self.api_key = str(api_key) if api_key is not None else ''

defaulting object construction to using the float (empty dict) handler will preserve float usage where expected

ericsomdahl avatar Dec 28 '17 02:12 ericsomdahl

Hello, thanks for implementing the fix. I feel that there needs to be a loud message about the dangers of using floats in high precision calculations.

Long ago i ran into the richard preyer superman half cent issue in an actual application, the effects become profound after several iterations of multiplications. Even today i fall now and then into this issue.

Here is an article on how using imprecise numerical representation caused 28 deaths and 100 injuries: http://www-users.math.umn.edu/~arnold/disasters/patriot.html

JosephMRally avatar Dec 28 '17 05:12 JosephMRally

Using floats for handling financial data is super dumb. It's a receipt for disaster.

maxmalysh avatar May 10 '18 17:05 maxmalysh