Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored NewsConn as discussed #13

Closed
wants to merge 4 commits into from

Conversation

erickmiller
Copy link
Contributor

@erickmiller erickmiller commented Sep 16, 2016

Refactored NewsConn to request comma delimited text (csv) instead of xml and added a request type to each news api function.

Did some bigger news text performance/benchmark comparison speed testing. Majority of results and information is in this thread: #9 (comment)

Also reintroduced mkt_tm bug fix (line 455) with slightly better logic (returning time.struct_time((0,0,0,0,0,0,0,0,0)) now instead of empty string) this issue (exception) keeps happening intermittently so this fixes it.

…Info, reverted mkt_tm potentially obsolete bug fix
…rmittent bug fix for mkt_tm issue (line 455) this bug keeps happening intermittently causing an exception, appears to come from dtn and dont think it would have an effect on live trading since it's a conn_stats attribute
@erickmiller
Copy link
Contributor Author

erickmiller commented Sep 17, 2016

Let me know if you have any more questions... but hoping this one makes it's way in.
I'll be here all weekend, leaving for London next week though so might be less available after the weekend for a bit.

@akapur
Copy link
Owner

akapur commented Sep 17, 2016

Could you please send me:

a) The python file you used of timing the news functionality.
b) A python file that results in the time field exception.

What OS (you mentioned Linux but which distro and version), what specific distribution of python (from the distro or something like anaconda). Are you running on a virtual machine somewhere or on bare metal hardware. Basically what precisely are you running on?

I’m getting different results and I want to see what you are doing.

Regarding the time field bug. Please run IQFeed.exe with the highest level of logging (read the IQFeed.exe developer docs for how to do this). pyiqfeed can send the appropriate messages to request this to IQFeed.exe so you don’t need to write any code. The log file will then contain all messages sent by you to IQFeed.exe and vice-versa. Edit this file and if there actually is a bug in IQFeed.exe, please file a bug report with DTN. They generally fix things like this in a week or less. I’m not seeing the issue and notwithstanding your confidence that it isn’t something that is likely to affect trading, I wouldn’t trade live with something like that happening. It may not be a market-data related message but it’s a symptom of something and you don’t know what that something is. Then again, you haven’t paid your “tuition” to the market yet so maybe I shouldn’t be making suggestions that prevent that from happening.

In the meantime, I’m not just “merging” your code. I have some time next week during which I will be refactoring the code and will likely use some but not all of your changes. The library will support news and real-time bars (not sure why you need them, you are normally better off subscribing to tick-data and doing filtering and bar creation if you need that yourself). Historical bars have always been there in HistoryConn.

Thanks.

Ashwin

On Sep 16, 2016, at 6:26 PM, Erick notifications@github.com wrote:

Let me know if you have any more questions... but hoping this one makes it's way in.
I'll be here all weekend, leaving for London next week though so might be less available after the weekend.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #13 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AAyQVPcd0pnnHAzQFdleR2alHn1BdnSdks5qq0HMgaJpZM4J-1md.

@erickmiller
Copy link
Contributor Author

erickmiller commented Sep 18, 2016

Could you please send me:
a) The python file you used of timing the news functionality.

Sure, will send later tonight am not in front of my testing workstation right now, but it's literally just each NewsConn api python call, one after the other on a big list of tickers, all wrapped with a time.time() on both sides. I'll send the python file when I'm back later.

b) A python file that results in the time field exception.

Yeah just use supplied example.py - it does it, but it is intermittent - I went half a day without it happening, and then it happened twice in a row one morning -- then didn't happen the third or fourth time, right afterwards, etc. But... you know what intermittent means. I could not trace it's origins to anything specific to the source code. I ran lots and lots of testing though and this bug primarily manifests at the very beginning of launch on the very first request and then appears to cease further leading me to believe it's something with iqfeed.

What OS (you mentioned Linux but which distro and version) what specific distribution of python

Ubuntu recently upgraded to 16.04, latest python 3.5 from the distro these particular tests are not from a virtual machine - I can send more specifics later when I'm back if you think it will help, like kernel version etc

Regarding the time field bug. Please run IQFeed.exe with the highest level of logging [...] I wouldn’t trade live with something like that happening. It may not be a market-data related message but it’s a symptom of something and you don’t know what that something is.

Really (really) good point. It could be a symptom of something else -- especially considering how all the data is coming from index ordered lists which feels fast but in the case one is missing could be fragile -- this is definitely something to look more into. Thanks for pointing out and I'll look at this closer when later tonight or tomorrow.

Then again [...] maybe I shouldn’t be making suggestions that prevent that from happening.

Suggestions are appreciated and encouraged. You seem to know your stuff. Are you a broker / dealer?

The library will support news and real-time bars (not sure why you need them [...]

I'm guessing when you say "them" you are referring to real-time bars and not news despite the ambiguity.

Anyway, yes I think you're 100% right here -- and in complete honestly, this class was a result of me being new to the esoteric details of the iqfeed TCP/IP API you have to admit, the IQFeed Docs are pretty misleading / confusing regarding getting quotes for derivatives and I misunderstood the nomenclature:
http://www.iqfeed.net/dev/api/docs/Derivatives_Overview.cfm

But to answer your question -- BarConn is far less useful once I realized that QuoteConn does everything needed including for all derivative symbols. I think the BarConn class could be useful in the future, plus it gives the API full coverage but that was not my intention when writing it. I thought QuoteConn wasn't working for derivatives -- because I tried it manually on 3 or 4 symbols that I copy/pasted and they just happened to not have any data for those dates so all I got back was !NO_DATA! exceptions (which I misconstrued as not working for derivatives) so I thought the section on "derivatives overview" was the solution, thus implementation of BarConn for the library, for free but not really fully needed by the implementer. It wasn't really that much code though and it did help familiarize with the ins and outs of this API so it was a helpful exercise and could be of potential use in the future.

In the meantime, I’m not just “merging” your code. I have some time next week during which I will be refactoring [...]

Umkay. Cool, your call. I'm in London next week so I definitely don't have time (or honestly much desire) to go back and forth too much more. I've run a pretty large number of tests on lots of tickers from SP500 and NASDAQ100 etc and the code is equally usable to the current code-base as-is right now IMHO. But, looking forward to your re-factoring updates am hoping it will be a positive and stable library wide release and improvement.

@akapur akapur closed this Oct 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants