oanda / v20-python

OANDA v20 bindings for Python
MIT License
225 stars 91 forks source link

Native data types #7

Closed alekna closed 8 years ago

alekna commented 8 years ago

API server returns decimal values as str. Should be float. Same problem with the streaming pricing.

>>> response = ctx.pricing.get(account_id, instruments='EUR_USD')
>>> pprint(json.loads(response.raw_body))
{'prices': [{'asks': [{'liquidity': 10000000, 'price': '1.11095'}],
             'bids': [{'liquidity': 10000000, 'price': '1.11082'}],
             'closeoutAsk': '1.11099',
             'closeoutBid': '1.11077',
             'instrument': 'EUR_USD',
             'quoteHomeConversionFactors': {'negativeUnits': '0.80307094',
                                            'positiveUnits': '0.80291619'},
             'status': 'tradeable',
             'time': '1478199233.623775422',
             'unitsAvailable': {'default': {'long': '0', 'short': '0'},
                                'openOnly': {'long': '0', 'short': '0'},
                                'reduceFirst': {'long': '0', 'short': '0'},
                                'reduceOnly': {'long': '0', 'short': '0'}}}]}
dmpettyp commented 8 years ago

We have chosen to represent all fixed point decimal numbers in the REST API as strings to ensure that precision delivered is exactly as intended. Internally we use a fixed point representation and do fixed point math. If a client wishes to convert the provided values into a floating point representation and perform math on them for their own models they are welcome to, but the over-the-wire representation for these values is a string.

alekna commented 8 years ago

I'm confused by the fact that minimumTradeSize, maximumPositionSize and maximumOrderUnits are decimal numbers. Do you allow trading fractional units on v20 engine?

{'displayName': 'EUR/USD',
 'displayPrecision': 5,
 'marginRate': '0.01',
 'maximumOrderUnits': '100000000',
 'maximumPositionSize': '0',
 'maximumTrailingStopDistance': '1.00000',
 'minimumTradeSize': '1',
 'minimumTrailingStopDistance': '0.00050',
 'name': 'EUR_USD',
 'pipLocation': -4,
 'tradeUnitsPrecision': 0,
 'type': 'CURRENCY'}
dmpettyp commented 8 years ago

Good observation. The v20 engine is capable of dealing with fractional units, however this is not a feature that OANDA is offering at this time.

alekna commented 8 years ago

I would like to get back to this issue again.

This is really inconvenient that you force using str instead of native data types. I'm not suggesting you should change your over-the-wire representation. Just wrap it properly within the wrapper. Higher level of abstraction is what wrapper is for. Isn't it?

Just had the following experience:

>>> ctx.order.create(account_id, order=ctx.order.LimitOrderRequest(instrument='EUR_USD', price=1.07903, units=1000000)).body
{'errorMessage': "Invalid value specified for 'order.units'"}
>>> 
>>> 
>>> ctx.order.create(account_id, order=ctx.order.LimitOrderRequest(instrument='EUR_USD', price=str(1.07903), units=str(1000000))).reason
'Created'

I would strongly suggest implementing native data types within the wrapper and then casting it to str whenever it's required on the server side.

dmpettyp commented 8 years ago

That sounds reasonable.

I'll also let you configure the context to allow for automatic conversion of the numeric types from string to their native type. This will likely be enabled by default.

alekna commented 8 years ago

In my opinion representing decimal numbers as strings creates unnecessary processing overhead on the client side. That's especially the case when processing streaming rates.

dmpettyp commented 8 years ago

you mean in the JSON wire format?

alekna commented 8 years ago

Yes. If someone wants to control the precision they can use displayPrecision for rounding or string formatting.

It's not GA yet, right? In that case you'd better change it earlier than later. Bids & asks are for math operations, not for printing as strings.

dmpettyp commented 8 years ago

Decimal numbers will always be rendered as strings over the wire as long as we are using JSON as the wire format. For example the difference between

{ "number" : 1.2345 }

and

{ "number" : "1.2345" }

over the wire is simply two "'s.

Regardless of the above representation, the client (either at the library or app level) will have to convert the string "1.2345" into a float. It's far too late to change the representation for this version of the API. I will be adding the option to convert to native types in the library though.

alekna commented 8 years ago

With 200-300 ticks/s that means 200-300 extra function calls and god knows how many extra CPU cycles per second. For what? For no reason. Just because you decided representation is more important than the data itself. It's NOT.

dmpettyp commented 8 years ago

I'm claiming that it's not an extra function call, it's a relocated function call. Converting from string -> float must always be done, either inside of the json.loads() call or after the json parsing (by the v20 library or the client code).

alekna commented 8 years ago

You might be right :)

alekna commented 8 years ago

When are you planning to add option to convert to native types in the library?

dmpettyp commented 8 years ago

i'm just doing some testing, should push it out tonight or tomorrow morning