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

Already on GitHub? Sign in to your account

Implemented ActiveParticipant for MtGox. #16

Merged
merged 2 commits into from Jan 31, 2013

Conversation

Projects
None yet
4 participants
Contributor

StevenLooman commented Jan 28, 2013

Implemented ActiveParticipant for MtGox.

I have touched some of the concept-classes. Please let me know if you have any comments and/or questions.

Contributor

StevenLooman commented Jan 28, 2013

This is also related to #6.

Contributor

StevenLooman commented Jan 29, 2013

Market has placeBidOrder/placeAskOrder to simplify the users' code. Now the user can simply do:

order =  market.placeBidOrder(5.0, 15.0)

Otherwise, the user has to create an Order first, with several tedious parameters such as the timestamp, or even parameters the user might now know the value of yet:

amount = Amount(5.0, BTC)
price = ExchangeRate(USD, BTC, 15.0)
timestamp = datetime.now()
order = Order(market, timestamp, Order.SELL, amount, price, entity=market)
updated_order = market.placeOrder(order)

This involves several more lines, and is also more error prone. Also, the user now has two orders: 1) the conceptual Order and 2) the MtGox specific order.

Feel free to move things around.

You are right about the Market.__private. It should be 'hidden.' I'll fix that soon.

repr is a repeating thing yes. Again, feel free to change things. You probably want to do something with dict, such as (not sure if this works):

return "<{0}({1})>".format(self.__class__.__name__, ", ".join(self.__dict__.values())

Or maybe add the keys of the dict as well.

Yes, good idea, we can Order.is_buy_order/is_sell_order. I'll commit this in a few minutes.

Contributor

StevenLooman commented Jan 29, 2013

Changed Order/is_buy_order/.is_sell_order.

Owner

goncalopp commented Jan 29, 2013

I didn't notice you were passing integers to placeOrder. Suddenly this makes more and less sense :)
The problem you mention is very, the user cannot be expected to do all that just to make a simple order.
OTOH, having such a simplified interface is making the assumption that those two values are all the data a market wants.

Let's say a new market accepts Orders into the future (it places the order only after some user-specified time). If Order was used, the user could just set the timestamp and be done. Even if the market supports more advanced order placing features, it's free to expose the in its Order subclass, which the user is free to use (or not at all, if he has no need for them). With this kind of interface (integers), it's impossible to expose such functionality.

So, is it possible to solve both the lazy-user and the rigid-interface problems?
What immediately comes to mind is using placeOrder( Order ) at this level and defining some kind of SimpleMarket, and let the user choose between being lazy and having advanced features. Most of pythons stdlib feels this way to me (socket, logging, urllib...)

I'll have to think a bit more about this, but I'll merge the request meanwhile

Contributor

StevenLooman commented Jan 30, 2013

Maybe we should add an additional parameter which accepts a dict, for market specific implementations. For example, using:

def placeBidOrder(self, amount, price, **args):
    ...

We can give additional parameters which, for example, the MtGox implementation can use. MtGox has an additional attribute called 'parameters' (from the top of my head) where you can specify an order is a limit or market order.

This solves the need for additional data, but is probably (also, compared to a placeOrder method) not very user friendly. Perhaps we can make it more user friendly by letting the user know - through API docs, or an additional constant at the MtGoxActiveParticipant, or both - what additional arguments can be given.

An additional contant at MtGoxActiveParticipant makes it possible for client-applications of the library/egg to read it and show additional options on the screen if needed.

@goncalopp goncalopp merged commit 37e15c4 into goncalopp:master Jan 31, 2013

Owner

goncalopp commented Jan 31, 2013

The merge is done. Sorry about the delay

As you mentioned, that solution solves the order customization. About the user knowing which arguments are available, there's nothing preventing, for example, MtgoxActiveParticipant of overriding placeBidOrder with default arguments:

def placeBidOrder( self, amount, price, is_limit_order=False ):

This still doesn't minimize the interface, though.

And another smallish problem: if, for any reason, a new mandatory parameter must be included on the base placeBidOrder or placeAskOrder, the API must be changed; any existing code using the API is now broken, and users are left wondering on the

TypeError: placeBidOrder() takes exactly N arguments (N-1 given)

If the parameters go on the order, there's no need to change the API (just throw an exception if there's not enough data, explaining what needs be included)

For now, I'll change this to placeOrder at this level. I'll be implementing some SimpleMarket, and see where it leads... Maybe it turns out to be a bad idea

Contributor

StevenLooman commented Feb 1, 2013

We can change MtGoxMarket.placeBidOrder to:

    def placeBidOrder( self, amount, price, **args):

Then, MtGoxMarket, for example, can check if args (dict) contains an entry with key 'order_type'. If not, it can raise an exception with a clear message that parameter 'order_type' is missing. ('order_type' is just an example.)

You can move it to Order, but that is just moving the same problem somewhere else. If you would place it on MtGoxOrder, the user still needs to understand that additional parameters are required.

Experimenting with it will give move insight. Be sure to do so.

What about the Bitstamp implementation? I can create an initial version for this using the updated concepts. I'm sure wel'll come across different different issues. Please let me know if you want me to do this.

@StevenLooman StevenLooman deleted the StevenLooman:mtgox_participant branch Feb 1, 2013

Owner

goncalopp commented Feb 3, 2013

I thought I had replied to this already, sorry. I did definitely read it...
main discussion on #13

You can move it to Order, but that is just moving the same problem somewhere else. If you would place it on MtGoxOrder, the user still needs to understand that additional parameters are required.

It's moving the problem somewhere else, yes, but it becomes a very different one. The market API needs be implemented by the exchanges; any change to it means changing ALL of them. Further, having that many options on placeOrder is a great way to turn ParticularExchange.placeOrder's implementation into a monolithic mess, since it can't easily pass the order to internal methods (there's so many arguments!).

If, OTOH, the optional arguments are passed on a single dict, it would be even less explicit than passing them as arguments on Order.

What about the Bitstamp implementation? I can create an initial version for this using the updated concepts. I'm sure wel'll come across different different issues. Please let me know if you want me to do this.

I think it's a bit early for that, since a lot of different ideas about the interface are still surfacing - but you're free to try, of course.

zuijlen commented Feb 3, 2013

Hi goncalopp,

...you're welcome to join the discussion.

Thank you.

It would be great to have a authoritative reference here...

The wiki link stated in my previous post. I bet each exchange has a different interface. Maybe the big FOREX interfaces/libraries can be used as reference ( http://en.wikipedia.org/wiki/Foreign_exchange_market ).
After some googling Oanda has some programs to interact with their api: http://fxtrade.oanda.com/trade-forex/api/
Dukascopy also has some api's:
Market order: http://www.dukascopy.com/wiki/#Set_Market_Order
Set Stop Loss Price: http://www.dukascopy.com/wiki/#Set_Stop_Loss_price
I have only limited knowledge of trading api's, and programming language, but I hope this (and probably some more references) should gives some more context.

This is a lot of new info; we'll have to consider everything you said carefully while designing the interface.

Thank you, this is my intention. To become a very good and widely used (multi currency) interface the library/interface should be able to adopt/extended with mentioned order variables in the future.
In the future.

I noticed some questions/discussion about the level of knowledge someone must have to be able to use the interface. If this concerns a 'must have' variable I would like to ask you to add the variable and set a default. For example: order type deafault = limit order (due to all interfaces require to add quantity and exchange price). And for the 'Time in force' default GTC.
This way a unfamiliar user gets the most safe, or most common, settings (they probably even expect it) and a professional user/exchange can select(/offer) a specific setting and do not turn down the interface because of some limitations.

I already post this in the other discussion (#13) and also here because #13 is already closed.

I think you meant ... self.high, self.low, self.last, ...

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