ENH: add interval kwarg to get_data_yahoo issue #9071 #9072
Conversation
jreback
added
Data Reader
IO Excel
|
needs some tests for this |
|
The Travis CI builds fail for an import error in an unrelated module. The PR pass all tests locally. @jreback OK |
|
you have to add tests to the PR |
|
Added tests for 'monthly', 'dividend' and invalid intervals in test_get_data_interval. |
|
@jreback I think the PR is ready for merge, the Travis CI failures "ERROR: Failure: ImportError (cannot import name json)" are unrelated to my change. All tests related to the pandas.io.data module pass on my machine with python2.7 and python3.4. Is there anything more I can do? |
|
pls add a release note (enhancements section, referencing the issue). rebase on master and you should be good. ping on green. |
|
|
||
| # dividend data | ||
| pan = web.get_data_yahoo('XOM', '2013-01-01', '2013-12-31', interval='v') | ||
| self.assertEqual(len(pan) == 4) |
There was a problem hiding this comment.
Maybe add tests for the other two interval possibilities?
There was a problem hiding this comment.
The Yahoo! CSV API is the same for 'd', 'w' and 'm' interval values, so testing one is enough to exercise all code in the function. The reason I added a second call with 'v' is that Yahoo! replies with a different CSV format in this case and that assures that it is handled by the same code without errors.
In my opinion it is not particularly useful to add more tests, but if you think it is necessary please tell me and I'll do it.
|
Hopefully I did the rebase correctly. I can't make sense of the Travis CI errors, some are unrelated to my changes and at least one appears to be a temporary failure of Yahoo! CSV API. I used the changes extensively a few days ago, so I'm quite confident of the code, but I experienced intermittent failures of the CSV API today. IMO the PR can be merged as is. Thanks. |
|
@alexamici well it appears to be failing on the routine you changed. |
|
@jreback short story: this is quite embarrassing, but the test function I wrote was total crap and I didn't notice as it was being skipped in my local testing. Now all should be well. Long story: to my partial excuse I usually use py.test assert or doctests so I had no prior experience with assertEqual and friends, and I made a mess of the test function. Furthermore when running locally my tests were skipped due to lxml missing and I didn't figure it out until today. Last but not least, I was using the 'GOOG' symbol that has been redefined recently and queries for 2013 resulted in a Yahoo! API failure. |
|
Tests are passing now! @jreback this is OK? |
ENH: add interval kwarg to get_data_yahoo issue #9071
|
thanks @alexamici ! |
|
btw, if you would like to review the docs in remote.rst for any updates |
PR for the issue #9071