-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
STY: use pytest.raises context manager (generic) #25603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,23 +102,34 @@ def test_nonzero_single_element(self): | |
s = Series([False]) | ||
assert not s.bool() | ||
|
||
msg = "The truth value of a Series is ambiguous" | ||
# single item nan to raise | ||
for s in [Series([np.nan]), Series([pd.NaT]), Series([True]), | ||
Series([False])]: | ||
pytest.raises(ValueError, lambda: bool(s)) | ||
with pytest.raises(ValueError, match=msg): | ||
bool(s) | ||
|
||
msg = "bool cannot act on a non-boolean single element Series" | ||
for s in [Series([np.nan]), Series([pd.NaT])]: | ||
pytest.raises(ValueError, lambda: s.bool()) | ||
with pytest.raises(ValueError, match=msg): | ||
s.bool() | ||
|
||
# multiple bool are still an error | ||
msg = "The truth value of a Series is ambiguous" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: Do we need to define this string 3 times in this file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
for s in [Series([True, True]), Series([False, False])]: | ||
pytest.raises(ValueError, lambda: bool(s)) | ||
pytest.raises(ValueError, lambda: s.bool()) | ||
with pytest.raises(ValueError, match=msg): | ||
bool(s) | ||
with pytest.raises(ValueError, match=msg): | ||
s.bool() | ||
|
||
# single non-bool are an error | ||
for s in [Series([1]), Series([0]), Series(['a']), Series([0.0])]: | ||
pytest.raises(ValueError, lambda: bool(s)) | ||
pytest.raises(ValueError, lambda: s.bool()) | ||
msg = "The truth value of a Series is ambiguous" | ||
with pytest.raises(ValueError, match=msg): | ||
bool(s) | ||
msg = "bool cannot act on a non-boolean single element Series" | ||
with pytest.raises(ValueError, match=msg): | ||
s.bool() | ||
|
||
def test_metadata_propagation_indiv(self): | ||
# check that the metadata matches up on the resulting ops | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fine to remove as this exact behaviour is tested on line 151 right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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 whereasobj == 0
does not.