csko / gdax-python-api

GDAX API written in Python3 using async/await
MIT License
52 stars 21 forks source link

use ujson over json because json.dumps() cannot serialize Decimal #7

Closed davluangu closed 6 years ago

davluangu commented 6 years ago

@csko I was running into errors with json.dumps() used here: https://github.com/davluangu/gdax-python-api/blob/76d9685a03e5f6404c13101c24c5cd45aec600b1/gdax/orderbook.py#L57 The issue is that json.dumps() cannot deal with Decimal() types. This is only an issue when using the trade_log_file_path arg in OrderBook().

Here's a toy example of the error:

import json
from decimal import Decimal

json.dumps({
    'bids': [[Decimal('849.6'), Decimal('0.002'), 'b06193b0-8a8e-47a9-9385-b9bzfa55d1db']],
    'asks': [[Decimal('849.61'), Decimal('0.00241623'), 'b8546721-3988-4210-9f83-e093fff86e1a']]    
})

output:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-afb1ff756b46> in <module>()
      8 }
      9
---> 10 json.dumps(book)

~/.pyenv/versions/3.6.3/lib/python3.6/json/__init__.py in dumps(obj, skipkeys, ensure_ascii, check_circular, allow_nan, cls, indent, separators, default, sort_keys, **kw)
    229         cls is None and indent is None and separators is None and
    230         default is None and not sort_keys and not kw):
--> 231         return _default_encoder.encode(obj)
    232     if cls is None:
    233         cls = JSONEncoder

~/.pyenv/versions/3.6.3/lib/python3.6/json/encoder.py in encode(self, o)
    197         # exceptions aren't as detailed.  The list call should be roughly
    198         # equivalent to the PySequence_Fast that ''.join() would do.
--> 199         chunks = self.iterencode(o, _one_shot=True)
    200         if not isinstance(chunks, (list, tuple)):
    201             chunks = list(chunks)

~/.pyenv/versions/3.6.3/lib/python3.6/json/encoder.py in iterencode(self, o, _one_shot)
    255                 self.key_separator, self.item_separator, self.sort_keys,
    256                 self.skipkeys, _one_shot)
--> 257         return _iterencode(o, 0)
    258
    259 def _make_iterencode(markers, _default, _encoder, _indent, _floatstr,

~/.pyenv/versions/3.6.3/lib/python3.6/json/encoder.py in default(self, o)
    178         """
    179         raise TypeError("Object of type '%s' is not JSON serializable" %
--> 180                         o.__class__.__name__)
    181
    182     def encode(self, o):

TypeError: Object of type 'Decimal' is not JSON serializable

The fix is quite simple, just use ujson (https://pypi.python.org/pypi/ujson) instead of json:

import ujson as json

Here's how you can recreate the actual error when using OrderBook(). It is basically the same code you have in main in orderbook.py with the uncommented line that sets trade_log_file_path:

import asyncio
from decimal import Decimal
from gdax.orderbook import OrderBook

async def run_orderbook():  # pragma: no cover
    async with OrderBook(
        ['ETH-USD', 'BTC-USD'],
        api_key=None,
        api_secret=None,
        passphrase=None,
        trade_log_file_path='trades.txt',
    ) as orderbook:
        while True:
            message = await orderbook.handle_message()
            if message is None:
                continue
            product_id = message['product_id']

loop = asyncio.get_event_loop()
loop.run_until_complete(run_orderbook())
davluangu commented 6 years ago

@csko As a side note, I haven't been able to get tests to successfully run locally. They are also failing on Travis for python 3.6 with my last PR (that was merged into master branch): https://travis-ci.org/csko/gdax-python-api/jobs/323521090

Any idea whats going on? I don't know enough about asyncio to really figure it out yet.

The first test failure (locally and on Travis) has this error:

E       TypeError: object MagicMock can't be used in 'await' expression
codecov[bot] commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@76d9685). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master      #7   +/-   ##
========================================
  Coverage          ?   96.2%           
========================================
  Files             ?       5           
  Lines             ?     501           
  Branches          ?       0           
========================================
  Hits              ?     482           
  Misses            ?      19           
  Partials          ?       0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 76d9685...b2578d7. Read the comment docs.

csko commented 6 years ago

Thanks for sending in this pull request. I'd prefer not introducing another dependency. Instead, this can be done with the cls= arg to json.dumps, see a2c2cb339ef4193ce62e17bb57367021246f359b. Also, the unit test was set up incorrectly because it was testing an empty orderbook, that's why this wasn't caught. It's fixed now.

The async tests were failing because they were not compatible with the newer release of asynctest. 4002b02388815df82b4a966cb16ab10875acab7c pins the package to the older version, CI should pass now. Make sure to run pip install -r requirements-dev.txt locally. I did not catch this because my local dev packages were older.

davluangu commented 6 years ago

@csko sounds good. thanks for the fix on the tests.