Skip to content

TST: add error message match for raise in test_datetimelike.py GH30999 #37952

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

Merged
merged 3 commits into from
Nov 20, 2020
Merged
Changes from 2 commits
Commits
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
26 changes: 21 additions & 5 deletions pandas/tests/arrays/test_datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import pandas as pd
import pandas._testing as tm
from pandas.core.arrays import DatetimeArray, PeriodArray, TimedeltaArray
from pandas.core.construction import array, extract_array
from pandas.core.indexes.datetimes import DatetimeIndex
from pandas.core.indexes.period import Period, PeriodIndex
from pandas.core.indexes.timedeltas import TimedeltaIndex
Expand Down Expand Up @@ -302,11 +303,26 @@ def test_searchsorted_castable_strings(self, arr1d, box):
expected = np.array([1, 2], dtype=np.intp)
tm.assert_numpy_array_equal(result, expected)

with pytest.raises(TypeError):
arr.searchsorted("foo")

with pytest.raises(TypeError):
arr.searchsorted([str(arr[1]), "baz"])
string = "foo"
with pytest.raises(
TypeError,
match=(
f"value should be a '{arr1d._scalar_type.__name__}', 'NaT', "
f"or array of those. Got '{type(string).__name__}' instead."
Copy link
Member

Choose a reason for hiding this comment

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

match takes a regular expression, which is checked using re.search. So the .s will match any character - can you use match=re.escape( instead of match=?

),
):
arr.searchsorted(string)
Copy link
Member

Choose a reason for hiding this comment

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

can this (and line 325) be kept as they originally were?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MarcoGorelli I see the point to keep it as original here. For line325, is it because extract_array() may change later and break the test? And in contract, StringArray (type name) is less likely to change.

Copy link
Member

Choose a reason for hiding this comment

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

It's not too important admittedly, it's just to minimise the amount of logic in tests as any assignment / substitution / branch has the potential to introduce bugs. When we write source code we want to write helper functions and not repeat ourselves and all of that, but for tests repetition is more acceptable - see https://testing.googleblog.com/2014/07/testing-on-toilet-dont-put-logic-in.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. The article is very helpful. Thanks a lot!


str_arr = [str(arr[1]), "baz"]
with pytest.raises(
TypeError,
match=(
f"value should be a '{arr1d._scalar_type.__name__}', 'NaT', "
f"or array of those. "
f"Got '{type(extract_array(array(str_arr))).__name__}' instead."
),
):
arr.searchsorted(str_arr)

def test_getitem_2d(self, arr1d):
# 2d slicing on a 1D array
Expand Down