Skip to content

Avoid bare pytest.raises in dtypes/test_dtypes.py #32672

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Mar 18, 2020
Merged

Avoid bare pytest.raises in dtypes/test_dtypes.py #32672

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1786b2d
Adding message check to pytest.raises for test_dtypes.py
Mar 11, 2020
98110e5
Merge branch 'master' of https://github.com/pandas-dev/pandas
Mar 11, 2020
bf777e8
Readding message check to pytest.raises for test_dtypes.py
Mar 11, 2020
2de67aa
Adding message check to pytest.raises for test_dtypes.py
Mar 11, 2020
e0dc7ac
Readding message check to pytest.raises for test_dtypes.py
Mar 11, 2020
7260d3e
Merge remote-tracking branch 'origin/30999_disallow_bare_pytest_raise…
Mar 13, 2020
ef61889
Added comment referencing an issue that may be related
Vlek Mar 13, 2020
1c17b5d
Parametrized the PeriodDType tests based on feedback
Mar 16, 2020
c7ffa14
Merge branch '30999_disallow_bare_pytest_raises_test_dtypes' of https…
Mar 16, 2020
8968337
Fixed the wording of the other two tests that used "categorial"
Mar 17, 2020
34c3b8f
Merge branch 'master' of https://github.com/pandas-dev/pandas into 30…
Mar 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pandas/core/dtypes/dtypes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ def validate_categories(categories, fastpath: bool = False):
if not fastpath:

if categories.hasnans:
raise ValueError("Categorial categories cannot be null")
raise ValueError("Categorical categories cannot be null")

if not categories.is_unique:
raise ValueError("Categorical categories must be unique")
Expand Down
22 changes: 13 additions & 9 deletions pandas/tests/dtypes/test_dtypes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def test_hash_vs_equality(self, dtype):
assert hash(dtype) == hash(dtype3)

def test_construction(self):
with pytest.raises(ValueError):
with pytest.raises(ValueError, match="Invalid frequency: xx"):
PeriodDtype("xx")

for s in ["period[D]", "Period[D]", "D"]:
Expand Down Expand Up @@ -414,16 +414,17 @@ def test_construction_from_string(self, dtype):
assert is_dtype_equal(dtype, result)
result = PeriodDtype.construct_from_string("period[D]")
assert is_dtype_equal(dtype, result)
with pytest.raises(TypeError):
msg = "Cannot construct a 'PeriodDtype' from "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is raised by pandas itself. In these cases, we would want to check the complete message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I left it out is because it's just repeating the input that we're giving it. If that's the case, then could I create a list of all of the strings and then for-loop through them? I don't know how much I should be changing the tests to add these messages so I was hesitant to go too far.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you can split and parametrize this tests could then do something like .. msg = f"Cannot construct a 'PeriodDtype' from '{string}'"

for guidance checkout test_construction_from_string tests for IntervalDtype (test_construction_from_string_errors) and DatetimeTZDtype (test_construct_from_string_invalid_raises) that have already been split and parametrized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, I added them using what you gave as a template. I also fixed the other two tests, everything passes.

with pytest.raises(TypeError, match=msg):
PeriodDtype.construct_from_string("foo")
with pytest.raises(TypeError):
with pytest.raises(TypeError, match=msg):
PeriodDtype.construct_from_string("period[foo]")
with pytest.raises(TypeError):
with pytest.raises(TypeError, match=msg):
PeriodDtype.construct_from_string("foo[D]")

with pytest.raises(TypeError):
with pytest.raises(TypeError, match=msg):
PeriodDtype.construct_from_string("datetime64[ns]")
with pytest.raises(TypeError):
with pytest.raises(TypeError, match=msg):
PeriodDtype.construct_from_string("datetime64[ns, US/Eastern]")

with pytest.raises(TypeError, match="list"):
Expand Down Expand Up @@ -475,7 +476,8 @@ def test_basic(self, dtype):

def test_empty(self):
dt = PeriodDtype()
with pytest.raises(AttributeError):
msg = "object has no attribute 'freqstr'"
with pytest.raises(AttributeError, match=msg):
str(dt)

def test_not_string(self):
Expand Down Expand Up @@ -764,11 +766,13 @@ def test_order_hashes_different(self, v1, v2):
assert c1 is not c3

def test_nan_invalid(self):
with pytest.raises(ValueError):
msg = "Categorical categories cannot be null"
with pytest.raises(ValueError, match=msg):
CategoricalDtype([1, 2, np.nan])

def test_non_unique_invalid(self):
with pytest.raises(ValueError):
msg = "Categorical categories must be unique"
with pytest.raises(ValueError, match=msg):
CategoricalDtype([1, 2, 1])

def test_same_categories_different_order(self):
Expand Down