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

MAINT: Sync and fill benchmarks through latest trading day #2044

Merged
merged 2 commits into from Dec 21, 2017

Conversation

Projects
None yet
3 participants
@ChrisPappalardo
Contributor

ChrisPappalardo commented Dec 6, 2017

It's analytically superior to have a trading system synced to the latest market close than to lag the market waiting for treasury benchmark data.

This PR removes a hard-wired 2-day lag and combines the timeseries of the market and treasury benchmarks before filling data forwards and backwards along the timeseries. This PR could be improved by averaging the treasury curve across points before backfilling the timeseries.

See comments to #2031 for more information.

@coveralls

This comment has been minimized.

coveralls commented Dec 6, 2017

Coverage Status

Coverage increased (+0.002%) to 87.374% when pulling 882a1a5 on ChrisPappalardo:cap-fix-benchmarks into 42b3200 on quantopian:master.

@freddiev4

Hi @ChrisPappalardo thanks for the PR! I just had one question for you below.

# combine dt indices and reindex using ffill then bfill
all_dt = br.index.union(tc.index)
br = br.reindex(all_dt, method='ffill').fillna(method='bfill')
tc = tc.reindex(all_dt, method='ffill').fillna(method='bfill')

This comment has been minimized.

@freddiev4

freddiev4 Dec 13, 2017

Contributor

The tradeoff here that I'm concerned about is that we'll end up having benchmark returns for much further back than 5 years (which is the max amount we can get from IEX), except the returns of that benchmark will just be the same b/c we're bfilling.

That's more memory we're using up and also then we have benchmark data that isn't accurate before the 5 year cutoff.

What is it that you're trying to get out of reindexing here?

This comment has been minimized.

@ChrisPappalardo

ChrisPappalardo Dec 14, 2017

Contributor

I'm trying to sync the two time series.

The Federal Reserve H15 report is not released on a timely basis with market closes, so there will always be missing data on the front-end of the treasury curve time series. At the same time, there are dates missing from both time series relative to each other. Joining the time series, re-indexing, and then filling the missing data solves both issues.

If the concern is filling data back past 5 years on the benchmark, you could add a line to drop all dates older than the most recent start of the two respective time series.

This comment has been minimized.

@freddiev4

freddiev4 Dec 14, 2017

Contributor

OK. That makes sense to me and sounds fair. Also just did some quick profiling and mem. usage wasn't what I had thought it would be like.

Can you amend your commit to use a Commit Prefix? I think MAINT: would be appropriate here, with a shorter commit message. Should be good to merge after that 🙂

This comment has been minimized.

@ChrisPappalardo

ChrisPappalardo Dec 16, 2017

Contributor

@freddiev4 this PR is good to go

@ChrisPappalardo ChrisPappalardo force-pushed the ChrisPappalardo:cap-fix-benchmarks branch from 882a1a5 to eb3bd95 Dec 15, 2017

@coveralls

This comment has been minimized.

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.002%) to 87.547% when pulling eb3bd95 on ChrisPappalardo:cap-fix-benchmarks into fdfce9b on quantopian:master.

@coveralls

This comment has been minimized.

coveralls commented Dec 21, 2017

Coverage Status

Coverage increased (+0.002%) to 87.557% when pulling 43dced3 on ChrisPappalardo:cap-fix-benchmarks into 2b07334 on quantopian:master.

@freddiev4 freddiev4 changed the title from Fill lagging treasury benchmark data through latest close to MAINT: Sync and fill benchmarks through latest trading day Dec 21, 2017

@freddiev4

This comment has been minimized.

Contributor

freddiev4 commented Dec 21, 2017

Hmm I thought I had merged this after seeing the email notification... 🤔

Anyways awesome work @ChrisPappalardo. Thanks for the PR 😃

@freddiev4 freddiev4 merged commit e4f555f into quantopian:master Dec 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment