Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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