TST: Change test_html to use stored data + mark other #4009

jtratner · 2013-06-24T01:52:30Z

Changed double decorated tests previously network and slow so they actually get run + stored usda_data such that the tests don't fail on network access.

cpcloud · 2013-06-24T03:30:23Z

thanks must have missed that when i wrote them. i think slow should be removed since either slow or network tests get run on travis iirc

cpcloud · 2013-06-24T03:32:23Z

also there's already spam.html in pandas/io/tests/data. i explicitly want to connect to the network here just in case something has changed, although there's no reason to believe the composition of spam will change anytime soon. so probably good to use spam.html

jtratner · 2013-06-24T03:34:04Z

Okay, I'll make that change. Do you want to store data locally for these or leave with the network request? html file is 32K

cpcloud · 2013-06-24T03:36:14Z

the banklist data set for example is updated every monday and so it's basically frozen in time and you cannot compare the data obtained with that that is stored locally. i get 28k (for spam) but that's splitting hairs... probably ok to leave local

cpcloud · 2013-06-24T03:37:57Z

i did du -h $(find . -name 'spam.html')

jtratner · 2013-06-24T03:39:20Z

Huh? Currently it's not local for the html file test. (most of the other test cases are). Plus these are basically just checks that lxml and BeautifulSoup actually work as expected.

jtratner · 2013-06-24T03:39:28Z

Huh? Currently it's not local for the html file test. (most of the other test cases are). Plus these are basically just checks that lxml and BeautifulSoup actually work as expected.

cpcloud · 2013-06-24T03:40:19Z

it's ok to change to local if u want that's what i meant

jtratner · 2013-06-24T03:40:39Z

Sorry for the double comment there - poor connection. The html file that the lxml and bs4 tests check for is 32k.

jtratner · 2013-06-24T03:41:20Z

Anyways I don't have a strong opinion. I'll edit this to remove slow soon.

cpcloud · 2013-06-24T03:43:16Z

oh u mean the one u added? or the one that is received by urlopen? i'm confused....all i'm saying is that if you want to change to use the local files instead of calling the network you can...they are there, spam is called spam.html and the banklist data set is called banklist.html...make sense?

jtratner · 2013-06-24T04:15:50Z

@cpcloud yeah, that does. just wondering if you think it's worth it - sorry I wasn't clear earlier

cpcloud · 2013-06-24T13:32:16Z

Since these are essentially smoke tests for the backend you're right best to move to slow decorators and remove the network decorators.

jtratner · 2013-06-24T22:02:12Z

one big reason why these take so long is that they are processing a huge
file. Is that necessary or can I reduce it to just the relevant portion
(i.e., <htm><head></head><body><table>TABLESTUFFHERE</table></body>)?

On Mon, Jun 24, 2013 at 9:32 AM, Phillip Cloud notifications@github.comwrote:

Since these are essentially smoke tests for the backend you're right best
to move to slow decorators and remove the network decorators.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4009#issuecomment-19906757
.

cpcloud · 2013-06-24T22:04:53Z

wanted to test on possibly invalid html from the wild. if you can find a smaller file that works that's great too

jtratner · 2013-06-25T02:45:49Z

@cpcloud hmm...well, just going to leave it as slow I guess. Sort of a bummer that it can't be both.

cpcloud · 2013-06-25T02:51:11Z

if you come across another data set in the wild please put up a PR, would love to have both just don't have time to check right now for data sets from pages that are both small and rife with HTML errors. this scraping framework might be of use for finding all kinds of different HTML tables.

jtratner · 2013-06-25T03:03:24Z

@cpcloud Changed double decorated tests previously network and slow so they actually get run + stored usda_data such that the tests don't fail on network access. The html file is 32k, not sure whether that's too large.

cpcloud · 2013-06-25T03:07:45Z

i'm confused..why not just use the spam data set that's already there?

jtratner · 2013-06-25T03:23:06Z

@cpcloud ohh, good point. Somehow I thought there was some decision to use a different file.

jtratner · 2013-06-25T03:26:24Z

changed it.

cpcloud · 2013-06-25T03:27:28Z

pandas/io/tests/test_html.py

-    url = ('http://ndb.nal.usda.gov/ndb/foods/show/1732?fg=&man=&'
-           'lfacet=&format=&count=&max=25&offset=&sort=&qlookup=spam')
-    assert get_lxml_elements(url, 'tbody')
+    filepath = "file://" + os.path.join(DATA_PATH, "spam.html")


is the explicit file protocol necessary here?

yes...doesn't work otherwise. (because it gets passed directly to the functions)

you know what can u change the get_lxml_elements to take a kwarg called base_url='file://' then pass that to the parse call as base_url like so: parse(url, base_url=base_url) then you don't have to concat with strings

cpcloud · 2013-06-27T01:53:16Z

@jreback look ok? i think this can be merged

jreback · 2013-06-27T02:21:27Z

looks ok

jtratner · 2013-06-27T02:48:37Z

Does this need rebase?

jtratner · 2013-06-29T01:56:40Z

@cpcloud more fun with network failures! :D https://travis-ci.org/jtratner/pandas/jobs/8559725 [pretty sure this is off of master and not due to this commit]

cpcloud · 2013-06-29T02:18:29Z

darn. that is off of master. i'm not even sure what the issue is there. i'll delve deeper. i really wish i had a way to reproduce this stuff. i may end up just setting up my own CI on my machines at home

jtratner · 2013-06-29T02:20:45Z

@cpcloud Maybe you could set up something that tracks all connections open at any one time (maybe mod urlopen to do it?) then, when it raises an error, have it print the entire set of open network connections.

cpcloud · 2013-06-29T14:55:29Z

there is no hope. will just catch this exception...teh interwebz r tricky

jtratner · 2013-06-29T14:57:17Z

@cpcloud that seems like the best bet here. Given the confusing-ness of the error, definitely something that the datareaders should handle themselves.

cpcloud · 2013-06-29T14:58:37Z

badstatusline exceptions are just like throwing Exception: completely uninformative if you want to use it to handle things gracefully

jtratner · 2013-06-29T14:59:52Z

I don't know what you are talking about. BadStatusLine exceptions indicate
that there was a bad status line. Duh. ;P

On Sat, Jun 29, 2013 at 10:58 AM, Phillip Cloud notifications@github.comwrote:

badstatusline exceptions are just like throwing Exception: completely
uninformative if you want to use it to handle things gracefull

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4009#issuecomment-20231241
.

cpcloud · 2013-06-29T15:00:55Z

ha. they are so specific with their empty string error message too

jtratner · 2013-07-01T23:05:50Z

@cpcloud can this (or something like it) get merged soon? These tests failed loudly when no internet available...

cpcloud · 2013-07-01T23:06:57Z

Sure maybe later tonight if not early tmrw morning

cpcloud · 2013-07-02T13:03:34Z

@jtratner can u rebase? once travis passes i can merge. sorry this fell by the wayside a bit.

jtratner · 2013-07-02T16:49:59Z

@cpcloud I thought this was handled by your data.py edit, which is why I wasn't thinking about it - didn't even notice anything until I had the internet turned off :P

cpcloud · 2013-07-02T17:01:24Z

No problem. I discovered yet another failure in data.py but I'm waiting to see if can reproduce it before making an issue. I feel like there has to be a better--meaning more reliable--approach. I wonder if yahoo and or Google has an api for this stuff. More so than just urlopening the CSV.

+ Change up test decorators so no tests marked as network and slow.

jtratner · 2013-07-02T23:33:45Z

rebased ftw

TST: Change test_html to use stored data + mark other

cpcloud · 2013-07-03T12:50:56Z

thanks!

cpcloud reviewed Jun 25, 2013
View reviewed changes

TST: Change test_html to use stored data.

6bc060b

+ Change up test decorators so no tests marked as network and slow.

cpcloud added a commit that referenced this pull request Jul 3, 2013

Merge pull request #4009 from jtratner/add-network-label-to-test-html

0523dc6

TST: Change test_html to use stored data + mark other

cpcloud merged commit 0523dc6 into pandas-dev:master Jul 3, 2013

jtratner deleted the add-network-label-to-test-html branch September 21, 2013 20:11

Uh oh!

TST: Change test_html to use stored data + mark other #4009

TST: Change test_html to use stored data + mark other #4009

Uh oh!

Conversation

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 24, 2013

Uh oh!

cpcloud commented Jun 24, 2013

Uh oh!

jtratner commented Jun 25, 2013

Uh oh!

cpcloud commented Jun 25, 2013

Uh oh!

jtratner commented Jun 25, 2013

Uh oh!

cpcloud commented Jun 25, 2013

Uh oh!

jtratner commented Jun 25, 2013

Uh oh!

jtratner commented Jun 25, 2013

Uh oh!

cpcloud Jun 25, 2013

Choose a reason for hiding this comment

Uh oh!

jtratner Jun 25, 2013

Choose a reason for hiding this comment

Uh oh!

cpcloud Jun 25, 2013

Choose a reason for hiding this comment

Uh oh!

cpcloud commented Jun 27, 2013

Uh oh!

jreback commented Jun 27, 2013

Uh oh!

jtratner commented Jun 27, 2013

Uh oh!

jtratner commented Jun 29, 2013

Uh oh!

cpcloud commented Jun 29, 2013

Uh oh!

jtratner commented Jun 29, 2013

Uh oh!

cpcloud commented Jun 29, 2013

Uh oh!

jtratner commented Jun 29, 2013

Uh oh!

cpcloud commented Jun 29, 2013

Uh oh!

jtratner commented Jun 29, 2013

Uh oh!

cpcloud commented Jun 29, 2013

Uh oh!