Skip to content

Conversation

@Tobiaqs
Copy link

@Tobiaqs Tobiaqs commented Dec 26, 2017

Fix for #108

Copy link
Collaborator

@skyl skyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this have major implications for people who depend on this library? Will downstream software continue to work as it has?

@skyl
Copy link
Collaborator

skyl commented Dec 26, 2017

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'

@ericsomdahl
Copy link
Owner

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

@JosephMRally
Copy link

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

@maxmalysh
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants