Skip to content
This repository was archived by the owner on Feb 7, 2026. It is now read-only.

Fix For Obtaining Yahoo Historical Data - #52

Open
skillachie wants to merge 16 commits into
cgoldberg:masterfrom
skillachie:master
Open

Fix For Obtaining Yahoo Historical Data#52
skillachie wants to merge 16 commits into
cgoldberg:masterfrom
skillachie:master

Conversation

@skillachie

@skillachie skillachie commented Jun 12, 2017

Copy link
Copy Markdown

Fix to update API to get historical data based on recent changes made by Yahoo. Inspired by fix in pandas_reader using cookies

@skillachie

Copy link
Copy Markdown
Author

Strange.

Not sure why the assert fails in the Travis environment. Can you take a look locally.

screen shot 2017-06-11 at 11 23 20 pm

@johnjackoleary

Copy link
Copy Markdown

Tested locally, and seems to work for me.

@johnjackoleary johnjackoleary left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

Comment thread ystockquote.py
'cookie': cookies}


def get_historical_prices(symbol, start_date, end_date, interval='1d'):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I might be using this wrong, but can 'interval' only be a number of days? If so, maybe rename the variable to interval_days and have the input be an int? That will make it more clear how it's used.

@skillachie skillachie Jul 17, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

interval variable can take the following:

1d - https://finance.yahoo.com/quote/%5EGSPC/history?period1=1468731600&period2=1500267600&interval=1d&filter=history&frequency=1d

1mo - https://finance.yahoo.com/quote/%5EGSPC/history?period1=1468731600&period2=1500267600&interval=1mo&filter=history&frequency=1mo

1wk - https://finance.yahoo.com/quote/%5EGSPC/history?period1=1468731600&period2=1500267600&interval=1wk&filter=history&frequency=1wk

This determines if you would like to obtain data rolled up daily, weekly or monthly. Changing it to interval days might be misleading. However this library was used to obtain only daily data before so we could remove it as an argument.

What do you think?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh I see. Hmm... Either way seems fine. Maybe keep it because being able to request at a specific interval would be sweet :D although could you add a small docstring explaining the options?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No problem. Will add it and push the update. Any idea why Travis returns wrong assert?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done with the requested changes

@johnjackoleary johnjackoleary left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good. No clue on Travis... :/

Comment thread ystockquote.py
from urllib import urlencode
import sys
reload(sys)
sys.setdefaultencoding('utf8')

@nooperpudd nooperpudd Oct 10, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please don't reload sys to change the default encoding

@cgoldberg

Copy link
Copy Markdown
Owner

hmm.. travis still failing. I'd like to get this merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants