added tiingo csv response serialisation options#822
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #822 +/- ##
==========================================
- Coverage 64.67% 63.27% -1.40%
==========================================
Files 62 62
Lines 2848 2775 -73
Branches 305 306 +1
==========================================
- Hits 1842 1756 -86
- Misses 932 945 +13
Partials 74 74 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bashtage
left a comment
bashtage
left a comment
There was a problem hiding this comment.
Mostly looks good. Hard par tis testing it given the API key.
pandas_datareader/tiingo.py
Outdated
| If no value is provided, defaults to 5min. The minimum value is "1min". | ||
| Units in minutes (min) and hours (hour) are accepted. | ||
| Re-sample frequency. Format is #min/hour; e.g. "15min" or "4hour". | ||
| If no value is provided, defaults to 5min. The minimum value is\ |
There was a problem hiding this comment.
Can you remove to \ and fix spacing.
pandas_datareader/tiingo.py
Outdated
| "1min". | ||
| Units in minutes (min) and hours (hour) are accepted. | ||
| response_format : str, default 'json' | ||
| Format of response data returned by the underlying Tiingo REST API. |
There was a problem hiding this comment.
Is this accurate? Perhaps something like: "The format to use in the request tot he TIINGO API. "csv" results in smaller payload, less bandwidth, and may result in smaller charges for accessing the TIINGO API"
There was a problem hiding this comment.
Sounds good - added wording along those lines. This would affect bandwidth limits and implicitly charges.
pandas_datareader/tiingo.py
Outdated
| "environmental variable TIINGO_API_KEY." | ||
| ) | ||
| self.api_key = api_key | ||
| self.response_format = ( |
There was a problem hiding this comment.
You should raise ValueError if not one of the expected values.
|
Regarding testing. The API has free plan - it would be possible to setup dedicated key for automated tests ( https://api.tiingo.com/about/pricing ) |
|
Hi, Just a quick ping to check whether I am missing anything for this PR to get it merged for a release? |
3 participants
git diff upstream/master -u -- "*.py" | flake8 --diffblack --check pandas_datareader