HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Bitex - Cryptocurrency Exchange API Framework for Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
exchangecryptocurrencyapibitexforpythonframework

Problem

BitEx is a Python module I've been working on for a little over 9 months now, as a side project. It was published 6 months ago on GitHub, and as I edge closer to my 1.0 release, I wanted to take the opportunity to present my code on here, in order to straighten it out.

What it solves and offers

It's designed to eliminate the need to get into the gory details of REST APIs of crypto exchanges, and offer a homogeneous and intuitive interface for all supported APIs. It takes care of authentication procedures, and offers a standardized set of methods (with identical method signature) for all commonly used methods at an exchange (polling order book & tickers, placing and cancelling orders, amongst others), as well as all other specific methods (or as many as I had the time to implement thus far).

It comes, essentially, as two sub packages: bitex.api is the backend taking care of setting up http requests via the requests module, as well as handling authentication specifics. It can be seen as wrapper for requests and technically could be used all on its own to send and receive data to/from exchanges.

The other is bitex.interfaces, which offers the above mentioned homogenous, standardized methods for all implemented exchanges. In addition to offering identical method signatures, it also aims to standardize method's return values. As these can differ significantly from exchange to exchange, these methods take care of data formatting, via the help of formatters found in bitex.formatters and the return_json decorator.
It relies on bitex.api.

Why I am submitting this for review

Ever since I started this project, I've rewritten the base code several times significantly. It took me a long time to figure out how to lay out the structure (which is mostly due to my learning curve over the past year as a first year software development apprentice).

Over the past two months, however, I've become rather fond and proud of the current structure and deem it quite prese

Solution

There's a lot of code here, so I'm just going to review bitex.utils. You'll see that there's plenty here for one review. Maybe some of the other reviewers here at Code Review will review some of your other code.

-
There is no module docstring. What is the purpose of this module? What does it contain?

-
import requests is in the "Built-Ins" section but as far as I know this module is not built into Python, so it should be in the "Third-Party" section.

-
return_json has no docstring. What does it do? What is the meaning of the formatter argument? What does it return?

-
It seems to be some kind of decorator for a function that returns a requests.Response object that converts it to a function returning two arguments, the first of which is either None (if the response was an error, or if the response contained data that could not be decoded as JSON), or the decoded JSON (if the response was successfully decoded and formatter is None), or the result of formatter applied to the decoded JSON and the original arguments to the function (otherwise).

This seems like a very complicated specification to me, and I think I would find it hard to use, because if you want to pass a formatter argument you have to ensure that it takes exactly the same arguments as the function being wrapped, which is not always going to be convenient, surely?

But maybe you know better, in which case the docstring would be a good place to make the case for the utility of this function, by giving some examples.

-
When writing a decorator, it's a good idea to copy attributes from the original function to the decorated function, so that the latter has the same name, module, docstring and so on as the former. There is a built-in function functools.wraps that you can use to do this.

-
When you log a message, don't use the % operator to format the message, instead pass the format string and format arguments separately as described in the documentation. (This gives more flexibility to the logger, for example if logging is suppressed then the message may never need to be formatted.)

-
Instead of calling Logger.error and putting the exception into the message, like this:

except Exception as e:
    log.error("return_json(): Unexpected error while parsing json "
              "from %s: %s" % (r.request.url, e))


use Logger.exception instead:

except Exception:
    log.exception("return_json(): Unexpected error while parsing JSON "
                  "from %s", r.request.url)


This adds a traceback to the log.

-
In this code, it looks as if you are using raise_for_status to determine if the request succeeded:

try:
    r.raise_for_status()
except requests.HTTPError as e:
    log.error(...)


I think it would be simpler just to check the status:

if r.status != requests.codes.ok:
    log.error(...)

Code Snippets

except Exception as e:
    log.error("return_json(): Unexpected error while parsing json "
              "from %s: %s" % (r.request.url, e))
except Exception:
    log.exception("return_json(): Unexpected error while parsing JSON "
                  "from %s", r.request.url)
try:
    r.raise_for_status()
except requests.HTTPError as e:
    log.error(...)
if r.status != requests.codes.ok:
    log.error(...)

Context

StackExchange Code Review Q#148123, answer score: 6

Revisions (0)

No revisions yet.