Skip to content

Conversation

@ldirer
Copy link
Contributor

@ldirer ldirer commented Jun 9, 2017

Reference Issue

What does this implement/fix? Explain your changes.

A test was decorated with @ignore_warnings(RuntimeWarning).

I noticed that nose would not run it locally.

The lack of keyword argument meant that the decorator would not behave as expected.
It would consider RuntimeWarning as the callable and I think in the end the whole

@ignore_warnings(RuntimeWarning)
def test():
   return 'ok'

would turn into a loose RuntimeWarning with message <function 'test'>.

Also it was RuntimeError instead of RuntimeWarning.

Any other comments?

@ldirer ldirer changed the title Fix decorator called without kwarg that would prevent test from running. [MRG] Fix decorator called without kwarg that would prevent test from running. Jun 9, 2017
@lesteve
Copy link
Member

lesteve commented Jun 9, 2017

Maybe we should have an error if obj is neither None nor is callable. That would prevent errors like this ...

@lesteve
Copy link
Member

lesteve commented Jun 9, 2017

Nope that wouldn't because Warning is callable 😞 ...

@lesteve
Copy link
Member

lesteve commented Jun 9, 2017

This is fine to merge when CIs are green. Good catch !

@lesteve
Copy link
Member

lesteve commented Jun 9, 2017

OK I am going to merge this one without waiting for AppVeyor, thanks a lot @ldirer !

@lesteve lesteve merged commit 6451a09 into scikit-learn:master Jun 9, 2017
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
AishwaryaRK pushed a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants