-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(scripts): add workflow for updating the efficient frontier #107
base: master
Are you sure you want to change the base?
Conversation
return daily_returns.values, name | ||
|
||
|
||
def update_returns(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems odd this is called update_returns
when it really is getting and formatting data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, will change it to get_returns()
or process_returns()
MAX_DELAY = 3.0 | ||
MAX_RETRIES = 3 | ||
|
||
while True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why have this code in a while loop if it just runs once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah this was a dirty fix for the case when the status code != 200
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could potentially run into an infinite loop if the status code != 200. Probably a good idea to include a max retry count
df = df[start_date:end_date] | ||
|
||
# convert to daily returns | ||
daily_returns = (1 + df.apy) ** (1 / 365.2425) - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 1 / 365.2425
? Isn't it just ** 365
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah this is because the returns from defillama are measured in yearly values, so I converted them to daily by taking the 365.2425-th root
|
||
def main_chart(returns): | ||
# aggregate to yearly values | ||
T = 365.2425 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dupe line as L66. Probably can abstract into a separate function to be shared
frontier_chart, frontier_data = main_chart(data) | ||
|
||
# login to datapane | ||
dp.login(token=os.environ["DATAPANE_TOKEN"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it's good to login first before parsing the data. Had the script throwing KeyError: 'DATAPANE_TOKEN'
after waiting for half an hour
adds the entrypoint file and the functions to allow automatic updates of the efficient frontier visualization.
Currently only supports the stablecoin assets, will open another PR to support the other asset pools.