TST: Move explicit connectivity checks to decorator.

[jtratner]

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]

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

[cpcloud]

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

[cpcloud]

nvm i added one from here.

[jtratner]

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]

i aspire to document like u

[jtratner]

haha :)

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

On Sat, Jun 15, 2013 at 1:44 PM, Phillip Cloud notifications@github.comwrote:

i aspire to document like u

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

[jtratner]

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

[cpcloud]

yep sure thing

[cpcloud]

oh i c u r referring to these doctests

[cpcloud]

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]

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

[jtratner]

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" notifications@github.com 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
.

[cpcloud]

probably raise_on_error like the rest of pandas :)

[jreback]

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)

[jtratner]

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]

@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:

1. Very easy to go back to the standard network decorator.
2. Using this format means that what network does is much clearer.
3. 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]

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

otherwise ready 2 go?

[jtratner]

@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]

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]

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

[jreback]

@jtratner this supersedes #3893, right?

cc @glitpak

[jreback]

@jtratner the connectivity check is fine

[jtratner]

Yes on your question @jreback
On Jun 18, 2013 6:39 PM, "jreback" notifications@github.com 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]

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

[jreback]

need rebase (prob release notes conflict)

[cpcloud]

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

[jreback]

@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]

yes test examples would be nice

[jtratner]

@cpcloud @jreback look at the docstrings

[jreback]

@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]

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 notifications@github.com 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]

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

[jreback]

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

[cpcloud]

@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]

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

[cpcloud]

very much agreed 👍

[cpcloud]

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

[jtratner]

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]

small typo...

[cpcloud]

looks ok 2 me ... @jreback ?

[jreback]

yep go ahead and merge

[jreback]

in new release notes format ?

[cpcloud]

it is

[cpcloud]

oh but slight error

[cpcloud]

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

[jtratner]

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

[cpcloud]

EPIC WIN!

[jtratner]

haha :)

[jreback]

so.... @jtratner ready to go?

[jtratner]

Yep.
On Jun 20, 2013 9:04 PM, "jreback" notifications@github.com 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
.

[jreback]

boom!

thanks a lot
this is good stuff!