Skip to content

Conversation

@Olshansk
Copy link
Collaborator

@Olshansk Olshansk commented Jan 3, 2021

Current Usage:

from iexfinance.stocks.cache import *

prepare_cache(CacheMetadata(cache_path="store.h5", cache_type=CacheType.HDF_STORE))

get_historical_data(...)
get_historical_data(...)
get_historical_data(...)

Current limitations:

  • Only optimizes message count when querying newer data
    for the historical prices endpoint.

I looked at how the requests-cache module handles a global
variables/sessions, but it can not apply in the case
of iexfinance (without refactoring) because requests-cache
makes use of requests's global session, so a global
variable is used here instead.

TODOs:

  • Discuss the approach.
  • Add documentation on how to use.
  • Document the use-cases/limitations of the historical cache.

Extra changes:

  • Added typing to get_historical_data. This was only
    done because it was the original point of entry in my
    work and I can reverse this.

  • closes #xxxx

  • tests added / passed

  • passes black iexfinance

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

  • added entry to docs/source/whatsnew/vLATEST.txt

Current Usage:
```
from iexfinance.stocks.cache import *

prepare_cache(CacheMetadata(cache_path="store.h5", cache_type=CacheType.HDF_STORE))

get_historical_data(...)
get_historical_data(...)
get_historical_data(...)
```

Current limitations:
- Only optimizes message count when querying newer data
  for the historical prices endpoint.

I looked at how the `requests-cache` module handles a global
variables/sessions, but it can not apply in the case
of iexfinance (without refactoring) because `requests-cache`
makes use of `requests`'s global session, so a global
variable is used here instead.

TODOs:
- Discuss the approach.
- Add documentation on how to use.
- Document the use-cases/limitations of the historical cache.

Extra changes:
- Added typing to get_historical_data. This was only
  done because it was the original point of entry in my
  work and I can reverse this.
@Olshansk Olshansk requested a review from addisonlynch January 3, 2021 20:15
@addisonlynch
Copy link
Owner

addisonlynch commented Jan 4, 2021

Initial thoughts. Going to take a more in-depth look early this week.

Looks good!

What are your thoughts about allowing the user to pass the cache as an argument to get_historical_data rather than using global variables?

e.g.

cache = prepare_cache(CacheMetadata(cache_path="store.h5", cache_type=CacheType.HDF_STORE))
get_historical_data("AAPL", start=start, end=end, cache=cache)

and then some logic in get_historical_data:

if cache is not None:
    return HistoricalReaderCache(
        symbols, start=start, end=end, close_only=close_only, cache=cache **kwargs
    ).fetch()
else:
   return HistoricalReader(
        symbols, start=start, end=end, close_only=close_only, **kwargs
    ).fetch()
  • iexfinance.stocks.cache --> iexfinance.caching (in the unlikely case we implement caching for an endpoint which does not fall under stocks)
  • Is functionality needed to partially or fully purge the cache? Probably not at the start, but something to consider

Trivial:

  • Do we need to worry about python 3.5 compatibility (f strings, type hints, etc.)? I think some people still use 3.5 but I am fine to move on from it https://pypistats.org/packages/iexfinance
  • No need to use unittest in the tests. Everything can be collected by pytest


def _execute_iex_query(self, url):
if len(self.symbols) > 1:
raise InternalError("Not supported yet")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic can go in the constructor after super

result = result.loc[:, ["close", "volume"]]
return result

def _get_historical_data(self, symbol, start, end):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use fetch in the parent class instead of this method?

symbol, start=start, end=end, close_only=self.close_only, **self.kwargs
).fetch()

def _get_historical_data_cached(self, symbol):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fetch for the subclass.

cache_type: CacheType = CacheType.NO_CACHE

def prepare_cache(cache: Union[CacheMetadata, pd.HDFStore]):
global _IEXFINANCE_CACHE_
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment about global variables

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.

3 participants