STY: use pytest.raises context manager (generic) #25603
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25603 +/- ##
==========================================
+ Coverage 91.26% 91.26% +<.01%
==========================================
Files 173 173
Lines 52968 52968
==========================================
+ Hits 48339 48340 +1
+ Misses 4629 4628 -1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #25603 +/- ##
==========================================
+ Coverage 91.26% 91.26% +<.01%
==========================================
Files 173 173
Lines 52968 52968
==========================================
+ Hits 48339 48340 +1
+ Misses 4629 4628 -1
Continue to review full report at Codecov.
|
|
|
||
| # multiple bool are still an error | ||
| msg = "The truth value of a Series is ambiguous" |
There was a problem hiding this comment.
nitpick: Do we need to define this string 3 times in this file?
There was a problem hiding this comment.
fine for now...
this is surely an indication of tests that could be refactored to use more pytest idiom... but that'll be for a follow-up.
also the messages serve as additional documentation for the tests
|
|
||
| # invalid behaviors | ||
|
|
||
| obj1 = self._construct(shape=4, value=1) | ||
| obj2 = self._construct(shape=4, value=1) | ||
|
|
||
| def f(): | ||
| with pytest.raises(ValueError, match=msg): |
There was a problem hiding this comment.
This should be fine to remove as this exact behaviour is tested on line 151 right?
There was a problem hiding this comment.
@alimcmaster1 : I'm not sure that this should be removed. I think it was added for a reason. If it could be removed then the remaining part of the test would probably be redundant as well.
probably related to the fact that bool(obj == 0) raises whereas obj == 0 does not.
jreback
added
Testing
|
thanks @simonjayhawkins |
xref #24332