ENH: add freq parameter to _BaseReader #199
Conversation
added `freq` paramter `_BaseReader` and updated `def params` to accomodate monthly frequency data from the World Bank database.
|
Thanks, can you add tests? |
| @@ -42,13 +42,14 @@ class _BaseReader(object): | |||
| _chunk_size = 1024 * 1024 | |||
| _format = 'string' | |||
|
|
|||
| def __init__(self, symbols, start=None, end=None, | |||
| def __init__(self, symbols, start=None, end=None, freq=None, | |||
There was a problem hiding this comment.
raise ValueError if freq is not None or "M"
There was a problem hiding this comment.
It may be that the base datareader should accept all of the valid pandas/datetime freq strings ('H', 'M', 'Q', 'A',...) and the specific modules for wb or oecd etc should then determine what is suitable for the particular data source? For instance World Bank offers monthly data, quarterly data, and annual data for the various data sources.
There was a problem hiding this comment.
No need to care values which is not implemented ATM from user's point of view. Also, pls move freq to WorldBankReader as other readers don't use it.
NOTE: Some other readers have interval kw which also accept frequency-like. It may better to use the same word but not sure as I'm not native English speaker.
|
I don't know how to write tests but I will learn. Can you (@sinhrks) point me to where they need to be, what one looks like and I will work out what I have to do? |
|
You can find some examples here. |
…y and annual data from World Bank. also added test for monthly and quarterly.
|
I have reversed changes I made to the base data reader class and bundled it all in the world bank reader. I have added two test, one for monthly data and one for quarterly. Both currently pass on my machine (Python 3.5.1 from Anaconda 2.5.0 (64 bit) - Windows). Let me know if there is anything else needing attention. |
| return {'date': '{0}M{1:02d}:{2}M{3:02d}'.format(self.start.year, | ||
| self.start.month, self.end.year, self.end.month), | ||
| 'per_page': 25000, 'format': 'json'} | ||
| if self.freq == 'Q': |
updated if-statement to utilise if-elif-else style
|
I like but I'd convert what is your opinion about this @sinhrks ? |
|
No opinion for |
|
Travis failure looks unrelated to the change. Let me check... |
femtotrader
changed the title
femtotrader
changed the title
|
@sinhrks @femtotrader Any date for this PR be merged ? I actually need it :) |
|
Sorry, should have noticed that test failure has been fixed in #204. @kurtforrester can u rebase? |
|
@kurtforrester is this still needed/useful? |
added
freqparamter_BaseReaderand updateddef paramsto accomodate monthly frequency data from the World Bank database.closes #198