-
Notifications
You must be signed in to change notification settings - Fork 181
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
edit unittest so that it uses GOOGL instead of GOOG #39
Conversation
…o accepts datetime objects as arguments
Added datetime functionality so that the get_historical_prices function also accepts datetime objects as arguments. It is especially useful when using the ystockquote module with many dates. The only addition to the list of dependencies is the datetime module. |
outer dict keys are dates ('YYYY-MM-DD') | ||
""" | ||
|
||
if isinstance(start_date, datetime.date): |
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.
im kinda not crazy about type checking here. is there a more pythonic way to do this with duck typing and exceptions?
i'm not sure what the whitespace changes are in the diff? could you revert those and keep things PEP8ish. thanks! |
@@ -33,7 +33,7 @@ def test_pep8_conformance(self): | |||
class YStockQuoteTestCase(TestWithScenarios): | |||
|
|||
def test_get_all(self): | |||
symbol = 'GOOG' | |||
symbol = 'GOOGL' |
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 call on updating the ticker symbol.
Thanks for the PR! I just reviewd and mostly just some nitpicks.. but I'm happy to merge something once it gets cleaned up. let me know if anything wasn't clear or you'd like me to make any of the suggested changes to get this landed. cheers. |
Hi Corey, Happy holidays! Thanks for looking at the PR and for your suggestions - and Also, I had accidentally converted to tabs instead of spaces somewhere, so I've made a brand new PR, so let me know if anything else catches your eye. Cheers, On Sun, Dec 27, 2015 at 9:51 PM, Corey Goldberg [email protected]
|
oh.. in the future, you can just push more fixes to your existing PR when rework something.. no need to create a new PR unless you are separating the PR into several discrete pieces. Anyway,, we'll followup on #40 and I'll close this one. |
The unit test was edited to account for the stock split on April 2, 2014. The class of shares trading on or before April 2 (known as Class A), changed its trading symbol from GOOG to GOOGL. When the markets opened on April 3, these shares of stock began trading under the new, 5-letter symbol.
Source: https://support.sigfig.com/hc/en-us/articles/202586324-What-happened-during-the-Google-Stock-split-