-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TST: parameterize more tests #45131
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
TST: parameterize more tests #45131
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
cf91057
parameterize more
mroeschke 5e8d390
Merge remote-tracking branch 'upstream/master' into ref/parameterize2
mroeschke 550fb05
Another parameterize
mroeschke 1289f31
Anotha one
mroeschke 4efcd0f
One more
mroeschke a6ba3ea
Address comments
mroeschke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,17 @@ def adjust_negative_zero(zero, expected): | |
return expected | ||
|
||
|
||
def compare_op(series, other, op): | ||
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. something like this you might want o make a first class assert_* function (but ok for now) |
||
left = np.abs(series) if op in (ops.rpow, operator.pow) else series | ||
right = np.abs(other) if op in (ops.rpow, operator.pow) else other | ||
|
||
cython_or_numpy = op(left, right) | ||
python = left.combine(right, op) | ||
if isinstance(other, Series) and not other.index.equals(series.index): | ||
python.index = python.index._with_freq(None) | ||
tm.assert_series_equal(cython_or_numpy, python) | ||
|
||
|
||
# TODO: remove this kludge once mypy stops giving false positives here | ||
# List comprehension has incompatible type List[PandasObject]; expected List[RangeIndex] | ||
# See GH#29725 | ||
|
@@ -959,77 +970,54 @@ def test_frame_operators(self, float_frame): | |
assert (df + df).equals(df) | ||
tm.assert_frame_equal(df + df, df) | ||
|
||
# TODO: taken from tests.series.test_operators; needs cleanup | ||
def test_series_operators(self): | ||
def _check_op(series, other, op, pos_only=False): | ||
left = np.abs(series) if pos_only else series | ||
right = np.abs(other) if pos_only else other | ||
|
||
cython_or_numpy = op(left, right) | ||
python = left.combine(right, op) | ||
if isinstance(other, Series) and not other.index.equals(series.index): | ||
python.index = python.index._with_freq(None) | ||
tm.assert_series_equal(cython_or_numpy, python) | ||
|
||
def check(series, other): | ||
simple_ops = ["add", "sub", "mul", "truediv", "floordiv", "mod"] | ||
|
||
for opname in simple_ops: | ||
_check_op(series, other, getattr(operator, opname)) | ||
@pytest.mark.parametrize( | ||
"func", | ||
[lambda x: x * 2, lambda x: x[::2], lambda x: 5], | ||
ids=["multiply", "slice", "constant"], | ||
) | ||
def test_series_operators_arithmetic(self, all_arithmetic_functions, func): | ||
op = all_arithmetic_functions | ||
series = tm.makeTimeSeries().rename("ts") | ||
other = func(series) | ||
compare_op(series, other, op) | ||
|
||
_check_op(series, other, operator.pow, pos_only=True) | ||
@pytest.mark.parametrize( | ||
"func", [lambda x: x + 1, lambda x: 5], ids=["add", "constant"] | ||
) | ||
def test_series_operators_compare(self, comparison_op, func): | ||
op = comparison_op | ||
series = tm.makeTimeSeries().rename("ts") | ||
other = func(series) | ||
compare_op(series, other, op) | ||
|
||
_check_op(series, other, ops.radd) | ||
_check_op(series, other, ops.rsub) | ||
_check_op(series, other, ops.rtruediv) | ||
_check_op(series, other, ops.rfloordiv) | ||
_check_op(series, other, ops.rmul) | ||
_check_op(series, other, ops.rpow, pos_only=True) | ||
_check_op(series, other, ops.rmod) | ||
@pytest.mark.parametrize( | ||
"func", | ||
[lambda x: x * 2, lambda x: x[::2], lambda x: 5], | ||
ids=["multiply", "slice", "constant"], | ||
) | ||
def test_divmod(self, func): | ||
series = tm.makeTimeSeries().rename("ts") | ||
other = func(series) | ||
results = divmod(series, other) | ||
if isinstance(other, abc.Iterable) and len(series) != len(other): | ||
# if the lengths don't match, this is the test where we use | ||
# `tser[::2]`. Pad every other value in `other_np` with nan. | ||
other_np = [] | ||
for n in other: | ||
other_np.append(n) | ||
other_np.append(np.nan) | ||
else: | ||
other_np = other | ||
other_np = np.asarray(other_np) | ||
with np.errstate(all="ignore"): | ||
expecteds = divmod(series.values, np.asarray(other_np)) | ||
|
||
tser = tm.makeTimeSeries().rename("ts") | ||
check(tser, tser * 2) | ||
check(tser, tser[::2]) | ||
check(tser, 5) | ||
|
||
def check_comparators(series, other): | ||
_check_op(series, other, operator.gt) | ||
_check_op(series, other, operator.ge) | ||
_check_op(series, other, operator.eq) | ||
_check_op(series, other, operator.lt) | ||
_check_op(series, other, operator.le) | ||
|
||
check_comparators(tser, 5) | ||
check_comparators(tser, tser + 1) | ||
|
||
# TODO: taken from tests.series.test_operators; needs cleanup | ||
def test_divmod(self): | ||
def check(series, other): | ||
results = divmod(series, other) | ||
if isinstance(other, abc.Iterable) and len(series) != len(other): | ||
# if the lengths don't match, this is the test where we use | ||
# `tser[::2]`. Pad every other value in `other_np` with nan. | ||
other_np = [] | ||
for n in other: | ||
other_np.append(n) | ||
other_np.append(np.nan) | ||
else: | ||
other_np = other | ||
other_np = np.asarray(other_np) | ||
with np.errstate(all="ignore"): | ||
expecteds = divmod(series.values, np.asarray(other_np)) | ||
|
||
for result, expected in zip(results, expecteds): | ||
# check the values, name, and index separately | ||
tm.assert_almost_equal(np.asarray(result), expected) | ||
|
||
assert result.name == series.name | ||
tm.assert_index_equal(result.index, series.index._with_freq(None)) | ||
for result, expected in zip(results, expecteds): | ||
# check the values, name, and index separately | ||
tm.assert_almost_equal(np.asarray(result), expected) | ||
|
||
tser = tm.makeTimeSeries().rename("ts") | ||
check(tser, tser * 2) | ||
check(tser, tser[::2]) | ||
check(tser, 5) | ||
assert result.name == series.name | ||
tm.assert_index_equal(result.index, series.index._with_freq(None)) | ||
|
||
def test_series_divmod_zero(self): | ||
# Check that divmod uses pandas convention for division by zero, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I dont think this improves the clarity of this test. id suggest something like:
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.
Hmm at least with this version an error with any of the
dtlike-ser
won't prevent the otherdtlike-ser
from being tested due to theparameterize
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.
Fair enough. I might be overly-wary of over-parametrizing in the arithmetic tests since there's a ton of them
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.
Hopefully with a second pass we can remove the need for this test entirely.