TST: Move explicit connectivity checks to decorator. #3914

jtratner · 2013-06-15T12:12:57Z

Instead, network decorator in pandas.util.testing catches IOError instead.
You have to opt into failing on tests by setting
pandas.util.testing._RAISE_NETWORK_ERROR_DEFAULT to True.

Also adds a with_network_connectivity_check that can automatically check for a connection.

Fixes #3910.

This version of the fix ignores all IOErrors and assumes there are connectivity problems
with any URLError.

jtratner · 2013-06-15T12:22:51Z

This version (jtratner/pandas@77ede4b) uses google.com as a proxy for network connectivity like the tests were previously doing.

cpcloud · 2013-06-15T16:28:30Z

@jtratner is there a assert raises warning function? i thought i remember u saying something about it somewhere...

cpcloud · 2013-06-15T17:38:57Z

nvm i added one from here.

jtratner · 2013-06-15T17:42:09Z

This is what I had before...just be aware that warnings seem kinda finicky (and you might want to remove the extra_warnings check).

@contextmanager
def assert_produces_warning(expected_warning=Warning, filter_level="always"):
    """
    Context manager for running code that expects to raise (or not raise)
    warnings.  Checks that code raises the expected warning and only the
    expected warning. Pass ``False`` or ``None`` to check that it does *not*
    raise a warning. Defaults to ``exception.Warning``, baseclass of all
    Warnings. (basically a wrapper around ``warnings.catch_warnings``).

    >>> import warnings
    >>> with assert_produces_warning():
    ...     warnings.warn(UserWarning())
    ...
    >>> with assert_produces_warning(False):
    ...     warnings.warn(RuntimeWarning())
    ...
    Traceback (most recent call last):
        ...
    AssertionError: Caused unexpected warning(s): ['RuntimeWarning'].
    >>> with assert_produces_warning(UserWarning):
    ...     warnings.warn(RuntimeWarning())
    Traceback (most recent call last):
        ...
    AssertionError: Did not see expected warning of class 'UserWarning'.

    ..warn:: This is *not* thread-safe.
    """
    with warnings.catch_warnings(record=True) as w:
        saw_warning = False
        warnings.simplefilter(filter_level)
        yield w
        extra_warnings = []
        for actual_warning in w:
            if expected_warning and \
                    issubclass(actual_warning.category, expected_warning):
                saw_warning = True
            else:
                extra_warnings.append(actual_warning.category.__name__)
        if expected_warning:
            assert saw_warning, ("Did not see expected warning of class %r."
                                 % expected_warning.__name__)
            assert not extra_warnings, ("Caused unexpected warning(s): %r."
                                    % extra_warnings)

cpcloud · 2013-06-15T17:44:24Z

i aspire to document like u

jtratner · 2013-06-15T17:45:42Z

haha :)

Easy to test the basics when you put it into a doctest...

On Sat, Jun 15, 2013 at 1:44 PM, Phillip Cloud [email protected]:

i aspire to document like u

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

jtratner · 2013-06-15T17:47:57Z

@cpcloud if you use it, nice to stick it in util/testing so it can be used elsewhere.

cpcloud · 2013-06-15T17:53:51Z

yep sure thing

cpcloud · 2013-06-16T23:37:08Z

oh i c u r referring to these doctests

cpcloud · 2013-06-17T13:37:48Z

this would be nice 2 merge i think this might address the hanging of google testing and some of those annoying network errors that have been sporadically popping up on travis. @jreback anything to add?

jreback · 2013-06-17T13:47:37Z

pandas/util/testing.py

@@ -35,7 +38,7 @@

 N = 30
 K = 4
-
+_FORCE_NETWORK_ERROR = False


can this be done in the decorator itself? (as a parameter?)

Yes, if you want to do that it's possible... Just have to add another level
of wrapper :P what should the keyword argument be? raise_error?
On Jun 17, 2013 9:47 AM, "jreback" [email protected] wrote:

In pandas/util/testing.py:

@@ -35,7 +38,7 @@

N = 30

K = 4

+_FORCE_NETWORK_ERROR = False

can this be done in the decorator itself? (as a parameter?)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3914/files#r4724107
.

probably raise_on_error like the rest of pandas :)

raise_on_error=False would be my recommend, so a 'normal' network error will invoke a skip test,otherwise raise (and if you are actually testing the network connectivity code, you can make this True)

Don't merge this quite yet - pushed an extra commit to see if I could get
at the error this and @cpcloud 's build hit... But then it passed :-/

jtratner · 2013-06-18T03:57:08Z

@jreback @cpcloud Okay, I added optional raise_on_error to network. I decided at this point to also implement a with_connectivity_check decorator to abstract out the remaining connectivity checks in the test cases + allow authors to choose to rely on the check for a URLError explicitly. To do this, I also added an optional_args decorator for decorators that lets you easily write decorators with optional arguments.

I hooked raise_on_error to a global _RAISE_NETWORK_ERROR_DEFAULT so that you can change it in one place and change it wherever else (obviously this doesn't work after the file is imported).

A few points:

Very easy to go back to the standard network decorator.
Using this format means that what network does is much clearer.
with_connectivity_check could potentially check FIRST and skip the test if it fails (and likely would need to test at end too). It's not particularly difficult to do this...I'm fine either way.

jreback · 2013-06-18T13:22:17Z

@jtratner need to rebase 1 more time as we just merged stuff (just release notes conflict)...

otherwise ready 2 go?

jtratner · 2013-06-18T13:38:08Z

@jreback I'll do it after work - need to remove a test snippet I added to try to get at a weird build failure - check out this build - https://travis-ci.org/jtratner/pandas/builds/8187252 - @cpcloud had the same error on a recent build too, so it's not the decorator (and wouldn't be caught by the decorator anyways). I'm puzzled by what set of circumstances causes that test to fail in that way.

jreback · 2013-06-18T14:01:52Z

I think that error might be that you have a 0-len series, which you are then indexing...

(e.g. the call failed but it didn't raise, rather returned something)

In [18]: df = DataFrame(columns=list('ABC'),index=date_range('20130101',periods=0))

In [19]: df
Out[19]: 
Empty DataFrame
Columns: [A, B, C]
Index: []

In [20]: df.index
Out[20]: 
<class 'pandas.tseries.index.DatetimeIndex'>
Length: 0, Freq: D, Timezone: None

In [21]: df['A'][-1]
IndexError: index out of bounds

jtratner · 2013-06-18T16:24:11Z

@jreback are you okay with the connectivity decorator? Don't want to add unnecessary complexity, but I think it's useful.

jreback · 2013-06-18T22:33:23Z

@jtratner this supersedes #3893, right?

cc @glitpak

jreback · 2013-06-18T22:39:34Z

@jtratner the connectivity check is fine

jtratner · 2013-06-18T22:43:59Z

Yes on your question @jreback
On Jun 18, 2013 6:39 PM, "jreback" [email protected] wrote:

@jtratner https://github.com/jtratner the connectivity check is fine

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

jtratner · 2013-06-19T01:31:48Z

@jreback this is totally ready to go now and now features a check_before_test option that might help @cpcloud ;)

jreback · 2013-06-19T01:34:41Z

need rebase (prob release notes conflict)

cpcloud · 2013-06-19T01:36:03Z

i was hoping that one of @gliptak /@jtratner's prs would fix the issue...if not i'll fix it

jreback · 2013-06-19T01:36:23Z

@jtratner as an aside this is something that would like to show how to use in a testing page
along with use of assert_catch_warnings

cpcloud · 2013-06-19T01:37:25Z

yes test examples would be nice

jtratner · 2013-06-19T01:40:23Z

@cpcloud @jreback look at the docstrings

jreback · 2013-06-19T01:44:51Z

@jtratner no that's what I meant

what we need is something that says here is the toolbox for testing

eg catch warnings - use this
assert equals - use this

almost like a style guide

jtratner · 2013-06-19T01:47:19Z

Okay, I can do that. I'm in the midst of adding all the Exception tests
anyways. Would it make sense to add that into the docs directly?

On Tue, Jun 18, 2013 at 9:45 PM, jreback [email protected] wrote:

@jtratner https://github.com/jtratner no that's what I meant

what we need is something that says here is the toolbox for testing

eg catch warnings - use this
assert equals - use this

almost like a style guide

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

jtratner · 2013-06-19T03:12:21Z

I believe this is all rebased now, I'll work on adding this to the wiki page.

jreback · 2013-06-19T11:27:16Z

@jtratner #3893 merged...you're on

cpcloud · 2013-06-20T18:31:47Z

@jtratner can u rebase? i'll run --with-timer on the google tests and see if i can't mark a few as slow that aren't yet marked. i'm a little lost in these yahoo google issues it seems like there are bunch of them floating around. any way we can consolidate some of them or maybe i just need to beef up my working memory a bit

jreback · 2013-06-20T18:33:51Z

@cpcloud on other note...let's just try to close out 0.11.1......I am going to stop moving things in......

cpcloud · 2013-06-20T18:34:21Z

very much agreed 👍

cpcloud · 2013-06-20T18:38:19Z

still need to rebase though :) gh is warning about merge conflicts

jtratner · 2013-06-20T20:55:30Z

this is now mergeable. If you commimt is, it will close the other issue too. Also, @cpcloud I'd prefer to merge this, then go ahead and try to figure out the test_google issues, esp because I'd like to use these decorators in the other exception refactor I'm doing, etc.

cpcloud · 2013-06-20T20:57:12Z

pandas/util/testing.py

+            ...
+        IOError: Failure Message
+
+    I you set check_before_test, it will check the url first and not run the test on failure::


small typo...

cpcloud · 2013-06-20T21:02:49Z

looks ok 2 me ... @jreback ?

jreback · 2013-06-20T21:05:56Z

yep go ahead and merge

jreback · 2013-06-20T21:06:18Z

in new release notes format ?

cpcloud · 2013-06-20T21:07:37Z

it is

cpcloud · 2013-06-20T21:08:02Z

oh but slight error

cpcloud · 2013-06-20T21:08:23Z

doc/source/release.rst

+    ``URLError`` as well). Added ``with_connectivity_check`` decorator to allow
+    explicitly checking a website as a proxy for seeing if there is network
+    connectivity. Plus, new ``optional_args`` decorator factory for decorators.
+    (:issue:`GH3910`, :issue:`GH3914`)


just write the issue # here, no need to write "GH"

TST: Enforce more strict rules about ignoring IOErrors with network tests ENH: Allow keyword arguments to network decorator ENH: skip try/except checks if raise_on_error

Instead, network decorator in pandas.util.testing checks for that instead. You have to opt into failing on tests by setting `pandas.util.testing._FORCE_NETWORK_ERROR` to `True`. CLN: Move imports and test skip to top of file

Allows tests to check that a url is available before bubbling up an error from a test case. TST: Change tests to use with_connectivity_check to better approximate previous behavior TST: Add check_before_test option to with_connectivity_check. CLN: PEP8 all the recent jratner code

jtratner · 2013-06-21T00:32:44Z

finally pushed it -- sorry bout that. with the new doc format...it's even able to rebase automatically which is great!

cpcloud · 2013-06-21T00:33:09Z

EPIC WIN!

jtratner · 2013-06-21T00:33:36Z

haha :)

jreback · 2013-06-21T01:04:21Z

so.... @jtratner ready to go?

jtratner · 2013-06-21T01:44:40Z

Yep.
On Jun 20, 2013 9:04 PM, "jreback" [email protected] wrote:

so.... @jtratner https://github.com/jtratner ready to go?

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

TST: Move explicit connectivity checks to decorator.

jreback · 2013-06-21T01:48:49Z

boom!

thanks a lot
this is good stuff!

jreback reviewed Jun 17, 2013
View reviewed changes

jreback mentioned this pull request Jun 18, 2013

Skip tests on network error #3893

Merged

jtratner mentioned this pull request Jun 20, 2013

pandas.io.tests.test_google:TestGoogle.test_get_data hangs during local testing #3916

Closed

cpcloud mentioned this pull request Jun 20, 2013

TST: Make network decorator impose a skipTest catching URLErrors #3910

Closed

cpcloud reviewed Jun 20, 2013
View reviewed changes

gliptak mentioned this pull request Jun 20, 2013

Change Finance Options signatures and deprecate year/month parameters #3822

Merged

jtratner added 4 commits June 20, 2013 20:31

TST: Change network decorator to auto-check for network errors

2295ce6

TST: Enforce more strict rules about ignoring IOErrors with network tests ENH: Allow keyword arguments to network decorator ENH: skip try/except checks if raise_on_error

TST: Remove explicit connectivity checks in test cases.

68fc014

Instead, network decorator in pandas.util.testing checks for that instead. You have to opt into failing on tests by setting `pandas.util.testing._FORCE_NETWORK_ERROR` to `True`. CLN: Move imports and test skip to top of file

DOC: Update docs with changes

7ddb586

jreback added a commit that referenced this pull request Jun 21, 2013

Merge pull request #3914 from jtratner/fix-network-using-tests

36c1263

TST: Move explicit connectivity checks to decorator.

jreback merged commit 36c1263 into pandas-dev:master Jun 21, 2013

jtratner deleted the fix-network-using-tests branch September 21, 2013 20:11

TST: Move explicit connectivity checks to decorator. #3914

TST: Move explicit connectivity checks to decorator. #3914

Conversation

jtratner commented Jun 15, 2013

jtratner commented Jun 15, 2013

cpcloud commented Jun 15, 2013

cpcloud commented Jun 15, 2013

jtratner commented Jun 15, 2013

cpcloud commented Jun 15, 2013

jtratner commented Jun 15, 2013

jtratner commented Jun 15, 2013

cpcloud commented Jun 15, 2013

cpcloud commented Jun 16, 2013

cpcloud commented Jun 17, 2013

jreback Jun 17, 2013

Choose a reason for hiding this comment

jtratner Jun 17, 2013

Choose a reason for hiding this comment

K = 4

cpcloud Jun 17, 2013

Choose a reason for hiding this comment

jreback Jun 17, 2013

Choose a reason for hiding this comment

jtratner Jun 18, 2013

Choose a reason for hiding this comment

jtratner commented Jun 18, 2013

jreback commented Jun 18, 2013

jtratner commented Jun 18, 2013

jreback commented Jun 18, 2013

jtratner commented Jun 18, 2013

jreback commented Jun 18, 2013

jreback commented Jun 18, 2013

jtratner commented Jun 18, 2013

jtratner commented Jun 19, 2013

jreback commented Jun 19, 2013

cpcloud commented Jun 19, 2013

jreback commented Jun 19, 2013

cpcloud commented Jun 19, 2013

jtratner commented Jun 19, 2013

jreback commented Jun 19, 2013

jtratner commented Jun 19, 2013

jtratner commented Jun 19, 2013

jreback commented Jun 19, 2013

cpcloud commented Jun 20, 2013

jreback commented Jun 20, 2013

cpcloud commented Jun 20, 2013

cpcloud commented Jun 20, 2013

jtratner commented Jun 20, 2013

cpcloud Jun 20, 2013

Choose a reason for hiding this comment

cpcloud commented Jun 20, 2013

jreback commented Jun 20, 2013

jreback commented Jun 20, 2013

cpcloud commented Jun 20, 2013

cpcloud commented Jun 20, 2013

cpcloud Jun 20, 2013

Choose a reason for hiding this comment

jtratner commented Jun 21, 2013

cpcloud commented Jun 21, 2013

jtratner commented Jun 21, 2013

jreback commented Jun 21, 2013

jtratner commented Jun 21, 2013

jreback commented Jun 21, 2013