search

csingley / ibflex

Python parser for Interactive Brokers Flex XML statements
MIT License
87 stars 43 forks source link

Added additional fields to class `Trade` #33

Closed pavitrakumar78 closed 3 years ago

pavitrakumar78 commented 3 years ago

Added new fields to reflect current IB's Trade flex query

csingley commented 3 years ago

Yes, this is how you do it. However, I'm not too sure about the types here... looking at the data you posted in your original bug report, it seems that fineness and weight are actually decimal numbers, and should be defined as such.

The types defined here also need to be harmonized with your test cases too... in the other pull request you checked in a test asserting that Trade.fineness is a decimal, but then here give it a type annotation that instructs the parser to convert this to a string. It's easy to predict that inconsistency is going to blow up in our faces.

It's not a big deal, but the usual convention is to check in the changes in the data model and the tests that exercise these changes at the same time, in the same pull request. The idea is that you never want to check in code that blows up your test suite (on the one hand) or that is untested (on the other hand).

deliveryType and commodityType are probably enums, I would guess, but until we get some actual data for these we should leave them as strings, as you have.

pavitrakumar78 commented 3 years ago

Sorry, my bad! fineness is indeed decimal, but weight is given as weight="0.0 ()" in the sample I provided. Would this still be classified as decimal?

csingley commented 3 years ago

weight="0.0 ()"

Yeah that's not good. It looks like this is metadata for a commodity, so the type here is most likely a decimal plus a unit of measure in parentheses... something like 136.5 (oz) or 2.34 (kg). It's just blank in your case because the commodity you're trading is dimensionless.

The ibflex library does not yet have types defined to deal with this sort of thing, and I'm certainly not going to run out and write them on spec without a bunch of hard data to fit. So I guess let's just leave the weight as type str for now.

Thank you.