Skip to content

BUG/TST: Yahoo Finance (a foundational resolve of GH: #2847, #2853) #2878

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

Closed
wants to merge 3 commits into from
Closed

BUG/TST: Yahoo Finance (a foundational resolve of GH: #2847, #2853) #2878

wants to merge 3 commits into from

Conversation

nehalecky
Copy link
Contributor

Resolve issues in pandas.io.data.py and test_yahoo.py.

@nehalecky
Copy link
Contributor Author

Hey @y-p, apologies for the wait on the fix. I'm not sure about my rebase with the branch, let me know if I can address anything!

Thanks!

@ghost
Copy link

ghost commented Feb 15, 2013

You removed the sanity checks, it's better to leave them in place and merely
add to them.

Not sure about removing @slow, because they are in fact slow.
Better keep test_fast.sh true to it's name.

I believe decode() uses the terminal encoding when no arg is passed in.
That's utf8 on most boxen but not all. notably non-english speaking windows.
So you should specify an encoding, yahoo returns utf8 doesn't it?

@nehalecky
Copy link
Contributor Author

Good tip on the sanity checks. I do like those, and I've already put them back in.

About the @slow, is 6.5 seconds slow for all 4 of the tests in test_yahoo.py? I substantially reduced the amount of data being queried in the background and that's the average time they take to complete now.

I believe the encoding is utf8, and I've added this and it seems to run fine, passing with flying colors.

Let me know about the @slow and I'll push the changes in single commit, thanks!

@ghost
Copy link

ghost commented Feb 16, 2013

thanks, cherry-picked, 44b2495 and friends.

@ghost ghost closed this Feb 16, 2013
@nehalecky
Copy link
Contributor Author

Great, thank you @y-p. Nice commit messages, btw. Helpful to learn from. Oh, and, what was your thoughts about 4 tests in 6.5 seconds? Would like to know for future reference.... Thanks!

@ghost
Copy link

ghost commented Feb 16, 2013

The tests included in not @slow average about 20ms each. So by comparison
these are slow. 4 seconds may not be a long time, I just opted for keeping the status-quo.

@nehalecky
Copy link
Contributor Author

Gotcha. Thanks for the info!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant