Skip to content

TST: Implement maximally-specific/strict fixtures for arithmetic tests #23734

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

Closed
wants to merge 6 commits into from
Closed

TST: Implement maximally-specific/strict fixtures for arithmetic tests #23734

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
86f3810
implement tm.to_array, maximally-specific fixtures
jbrockmendel Nov 16, 2018
375fdb4
Merge branch 'master' of https://github.com/pandas-dev/pandas into box
jbrockmendel Nov 16, 2018
f8a3988
implement box4b, box5b
jbrockmendel Nov 16, 2018
27c2ce1
Merge branch 'master' of https://github.com/pandas-dev/pandas into box
jbrockmendel Nov 17, 2018
df74b6b
Merge branch 'master' of https://github.com/pandas-dev/pandas into box
jbrockmendel Nov 17, 2018
9c07638
better names
jbrockmendel Nov 17, 2018
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
95 changes: 76 additions & 19 deletions pandas/tests/arithmetic/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,22 @@
import pandas as pd

from pandas.compat import long
from pandas.core.arrays import PeriodArray, DatetimeArrayMixin as DatetimeArray
import pandas.util.testing as tm


# ------------------------------------------------------------------
# Helper Functions

def id_func(x):
if isinstance(x, tuple):
assert len(x) == 2
return x[0].__name__ + '-' + str(x[1])
else:
return x.__name__


# ------------------------------------------------------------------

@pytest.fixture(params=[1, np.array(1, dtype=np.int64)])
def one(request):
# zero-dim integer array behaves like an integer
Expand Down Expand Up @@ -137,7 +150,7 @@ def mismatched_freq(request):
# ------------------------------------------------------------------

@pytest.fixture(params=[pd.Index, pd.Series, pd.DataFrame],
ids=lambda x: x.__name__)
ids=id_func)
def box(request):
"""
Several array-like containers that should have effectively identical
Expand All @@ -150,42 +163,86 @@ def box(request):
pd.Series,
pytest.param(pd.DataFrame,
marks=pytest.mark.xfail(strict=True))],
ids=lambda x: x.__name__)
ids=id_func)
def box_df_fail(request):
"""
Fixture equivalent to `box` fixture but xfailing the DataFrame case.
"""
return request.param


@pytest.fixture(params=[(pd.Index, False),
(pd.Series, False),
@pytest.fixture(params=[pd.Index, pd.Series, pd.DataFrame, tm.to_array],
ids=id_func)
def box4(request):
"""
Fixture to test on each of Index, Series, DataFrame, and pandas Array
classes.
"""
return request.param


@pytest.fixture(params=[pd.Index,
pd.Series,
(pd.DataFrame, False),
pytest.param((pd.DataFrame, True),
marks=pytest.mark.xfail(strict=True))],
ids=lambda x: x[0].__name__ + '-' + str(x[1]))
def box_transpose_fail(request):
(pd.DataFrame, True),
tm.to_array],
ids=id_func)
def box5(request):
"""
Fixture similar to `box` but testing both transpose cases for DataFrame,
with the tranpose=True case xfailed.
Like `box4`, but the DataFrame case with box transpose=True and
transpose=False
"""
# GH#23620
return request.param


@pytest.fixture(params=[pd.Index, pd.Series, pd.DataFrame, PeriodArray],
ids=lambda x: x.__name__)
def box_with_period(request):
@pytest.fixture(params=[pd.Index,
pd.Series,
(pd.DataFrame, False),
pytest.param((pd.DataFrame, True),
marks=pytest.mark.xfail(strict=True)),
tm.to_array],
ids=id_func)
def box5_tfail(request):
"""
Like `box`, but specific to PeriodDtype for also testing PeriodArray
Like `box5`, but xfailing the transposed DataFrame case
"""
# GH#23620
return request.param


@pytest.fixture(params=[pd.Index, pd.Series, pd.DataFrame, DatetimeArray],
ids=lambda x: x.__name__)
def box_with_datetime(request):
# Construct tuples of the form (box, tz) over all our box classes and
# the timezones in tz_naive_fixture. We then xfail the cases where the box
# is a DataFrame-with-transpose and the timezone is not tz-naive (i.e. None)
boxes = [pd.Index,
pd.Series,
(pd.DataFrame, False),
(pd.DataFrame, True),
tm.to_array]

# copied from pandas.conftest tz_naive_fixture
TIMEZONES = [None, 'UTC', 'US/Eastern', 'Asia/Tokyo', 'dateutil/US/Pacific',
'dateutil/Asia/Singapore']

params = [(x, y) for x in boxes for y in TIMEZONES]
for n in range(len(params)):
tup = params[n]
if isinstance(tup[0], tuple) and tup[0][1] is True and tup[1] is not None:
# i.e. (DataFrame, True, tzinfo) excluding the no-tzinfo case
param = pytest.param(tup, marks=pytest.mark.xfail(strict=True))
params[n] = param


@pytest.fixture(params=params)
Copy link
Contributor

Choose a reason for hiding this comment

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

so my comment above was fairly general in nature. I don't think you need to test tz's other than [None, 'UTC'] or maybe use 'US/Eastern' rather than UTC to make sure that no times are shifted.

you actually just care if there IS a timezone or not.

def box5_and_tz(request):
Copy link
Member

Choose a reason for hiding this comment

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

Do we think this is really needed? (all the timezones) Because with all combinations, you get a quite combinatorial explosion (using this makes a single test into 30 tests)
And if you then further combine it with other fixtures ..

(I don't know how much it is used)

Copy link
Member Author

Choose a reason for hiding this comment

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

combinatorial explosion is an entirely reasonable concern.

Note that this fixture is taking the place of existing usages of box[...], tz_naive_fixture, so the net effect on the number of cases is not as big as it might look at first glance. The big benefit is that tests that previously were either skipped or marked with a # FIXME are now explicitly+strictly xfailed. (some of these can be un-xfailed and simplified if/when the issue behind #23730 is fixed)

More generally the timezone fixtures may be leading to more tests than we really need/want, especially given the overhead of pytest collection. Two thoughts on this I've been mulling over:

  • We've gotten rid of a lot of for-loops and replaced them with parametrization/fixtures. Swinging back in the other direction may be worth considerin. Using the --showlocals flag will cause the pytest error messages to tell us where in the loop an error occurred, obviating one of the disadvantages of looping.

  • Instead of using a fixed set of timezone strings, have a smaller randomized set. We could randomize over the whole space of possible dateutil+pytz+stdlib tzinfos, giving corner cases fewer shadows in which to hide.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, in general those fixtures are certainly good.

There is for example one place where it is combined with the two_hours fixture (6 different ways for timedelta-like), giving 180 tests.

Maybe more specific for this case: are both 'US/Eastern' and 'Asia/Tokyo' needed? (did we had bugs in the past that were related to that?)
And the same for the dateutil ones: did we have bugs in the past in the arithmetics related to having a dateutil timezone?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea about why tz_naive_fixture includes exactly what it does.

If "the tests are unnecessarily thorough" is the biggest problem we have this week, it's a pretty darn good week.

Copy link
Contributor

Choose a reason for hiding this comment

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

so the timezones that are chosen, have been sources of (since fixed) bugs in the past. We have a pretty good representative sample of various edge cases on timezones and i think we have just about the right amount, not too few and not too many, after all its just 2 per timezone library type + UTC + None.

"""
Like `box`, but specific to datetime64 for also testing DatetimeArray
Fixture to test over Index, Series, DataFrame, and pandas Array, also
providing timezones, xfailing timezone-aware cases with transposed
DataFrame
"""
return request.param


# aliases so we can use the same fixture twice in a test
Copy link
Contributor

Choose a reason for hiding this comment

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

these need self-obvious names

box4b = box4
box5b = box5
68 changes: 28 additions & 40 deletions pandas/tests/arithmetic/test_datetime64.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,14 @@ def test_dt64_ser_cmp_date_warning(self):
assert "a TypeError will be raised" in str(m[0].message)

@pytest.mark.skip(reason="GH#21359")
def test_dt64ser_cmp_date_invalid(self, box_with_datetime):
def test_dt64ser_cmp_date_invalid(self, box5):
# GH#19800 datetime.date comparison raises to
# match DatetimeIndex/Timestamp. This also matches the behavior
# of stdlib datetime.datetime
box = box_with_datetime

ser = pd.date_range('20010101', periods=10)
date = ser.iloc[0].to_pydatetime().date()

ser = tm.box_expected(ser, box)
ser = tm.box_expected(ser, box5)
assert not (ser == date).any()
assert (ser != date).all()
with pytest.raises(TypeError):
Expand Down Expand Up @@ -230,13 +228,12 @@ def test_timestamp_compare_series(self, left, right):
result = right_f(pd.Timestamp("nat"), s_nat)
tm.assert_series_equal(result, expected)

def test_dt64arr_timestamp_equality(self, box_with_datetime):
def test_dt64arr_timestamp_equality(self, box4):
# GH#11034
box = box_with_datetime
xbox = box if box not in [pd.Index, DatetimeArray] else np.ndarray
xbox = box4 if box4 not in [pd.Index, DatetimeArray] else np.ndarray

ser = pd.Series([pd.Timestamp('2000-01-29 01:59:00'), 'NaT'])
ser = tm.box_expected(ser, box)
ser = tm.box_expected(ser, box4)

result = ser != ser
expected = tm.box_expected([False, True], xbox)
Expand Down Expand Up @@ -1071,10 +1068,10 @@ def test_dti_add_sub_float(self, op, other):
with pytest.raises(TypeError):
op(dti, other)

def test_dti_add_timestamp_raises(self, box_with_datetime):
def test_dti_add_timestamp_raises(self, box4):
# GH#22163 ensure DataFrame doesn't cast Timestamp to i8
idx = DatetimeIndex(['2011-01-01', '2011-01-02'])
idx = tm.box_expected(idx, box_with_datetime)
idx = tm.box_expected(idx, box4)
msg = "cannot add"
with pytest.raises(TypeError, match=msg):
idx + Timestamp('2011-01-01')
Expand Down Expand Up @@ -1186,21 +1183,17 @@ def test_dti_add_intarray_no_freq(self, box):
# -------------------------------------------------------------
# Binary operations DatetimeIndex and timedelta-like

def test_dti_add_timedeltalike(self, tz_naive_fixture, two_hours,
box_with_datetime):
def test_dti_add_timedeltalike(self, two_hours, box5_and_tz):
# GH#22005, GH#22163 check DataFrame doesn't raise TypeError
box = box_with_datetime

tz = tz_naive_fixture
box, tz = box5_and_tz
rng = pd.date_range('2000-01-01', '2000-02-01', tz=tz)

# FIXME: calling with transpose=True raises ValueError
rng = tm.box_expected(rng, box, transpose=False)
rng = tm.box_expected(rng, box)

result = rng + two_hours
expected = pd.date_range('2000-01-01 02:00',
'2000-02-01 02:00', tz=tz)
expected = tm.box_expected(expected, box, transpose=False)
expected = tm.box_expected(expected, box)
tm.assert_equal(result, expected)

def test_dti_iadd_timedeltalike(self, tz_naive_fixture, two_hours):
Expand All @@ -1227,18 +1220,16 @@ def test_dti_isub_timedeltalike(self, tz_naive_fixture, two_hours):
rng -= two_hours
tm.assert_index_equal(rng, expected)

def test_dt64arr_add_sub_td64_nat(self, box, tz_naive_fixture):
def test_dt64arr_add_sub_td64_nat(self, box5_and_tz):
# GH#23320 special handling for timedelta64("NaT")
tz = tz_naive_fixture
box, tz = box5_and_tz

dti = pd.date_range("1994-04-01", periods=9, tz=tz, freq="QS")
other = np.timedelta64("NaT")
expected = pd.DatetimeIndex(["NaT"] * 9, tz=tz)

# FIXME: fails with transpose=True due to tz-aware DataFrame
# transpose bug
obj = tm.box_expected(dti, box, transpose=False)
expected = tm.box_expected(expected, box, transpose=False)
obj = tm.box_expected(dti, box)
expected = tm.box_expected(expected, box)

result = obj + other
tm.assert_equal(result, expected)
Expand Down Expand Up @@ -1473,13 +1464,13 @@ def test_sub_dti_dti(self):
tm.assert_index_equal(result, expected)

@pytest.mark.parametrize('dti_freq', [None, 'D'])
def test_dt64arr_add_sub_period(self, dti_freq, box_with_datetime):
def test_dt64arr_add_sub_period(self, dti_freq, box5):
# GH#13078
# not supported, check TypeError
p = pd.Period('2011-01-01', freq='D')

idx = pd.DatetimeIndex(['2011-01-01', '2011-01-02'], freq=dti_freq)
idx = tm.box_expected(idx, box_with_datetime)
idx = tm.box_expected(idx, box5)

with pytest.raises(TypeError):
idx + p
Expand All @@ -1492,14 +1483,13 @@ def test_dt64arr_add_sub_period(self, dti_freq, box_with_datetime):

@pytest.mark.parametrize('pi_freq', ['D', 'W', 'Q', 'H'])
@pytest.mark.parametrize('dti_freq', [None, 'D'])
def test_dti_add_sub_pi(self, dti_freq, pi_freq,
box_with_datetime, box_with_period):
def test_dti_add_sub_pi(self, dti_freq, pi_freq, box4, box4b):
# GH#20049 subtracting PeriodIndex should raise TypeError
dti = pd.DatetimeIndex(['2011-01-01', '2011-01-02'], freq=dti_freq)
pi = dti.to_period(pi_freq)

dtarr = tm.box_expected(dti, box_with_datetime)
parr = tm.box_expected(pi, box_with_period)
dtarr = tm.box_expected(dti, box4)
parr = tm.box_expected(pi, box4b)

with pytest.raises(TypeError):
dtarr + parr
Expand Down Expand Up @@ -1828,24 +1818,22 @@ def test_dti_with_offset_series(self, tz_naive_fixture, names):
res3 = dti - other
tm.assert_series_equal(res3, expected_sub)

def test_dti_add_offset_tzaware(self, tz_aware_fixture, box_with_datetime):
def test_dti_add_offset_tzaware(self, box5_and_tz):
# GH#21610, GH#22163 ensure DataFrame doesn't return object-dtype
box = box_with_datetime
box, tz = box5_and_tz

timezone = tz_aware_fixture
if timezone == 'US/Pacific':
dates = date_range('2012-11-01', periods=3, tz=timezone)
if tz == 'US/Pacific':
dates = date_range('2012-11-01', periods=3, tz=tz)
offset = dates + pd.offsets.Hour(5)
assert dates[0] + pd.offsets.Hour(5) == offset[0]

dates = date_range('2010-11-01 00:00',
periods=3, tz=timezone, freq='H')
periods=3, tz=tz, freq='H')
expected = DatetimeIndex(['2010-11-01 05:00', '2010-11-01 06:00',
'2010-11-01 07:00'], freq='H', tz=timezone)
'2010-11-01 07:00'], freq='H', tz=tz)

# FIXME: these raise ValueError with transpose=True
dates = tm.box_expected(dates, box, transpose=False)
expected = tm.box_expected(expected, box, transpose=False)
dates = tm.box_expected(dates, box)
expected = tm.box_expected(expected, box)

# TODO: parametrize over the scalar being added? radd? sub?
offset = dates + pd.offsets.Hour(5)
Expand Down
29 changes: 11 additions & 18 deletions pandas/tests/arithmetic/test_period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,36 +553,31 @@ def test_pi_sub_isub_offset(self):
rng -= pd.offsets.MonthEnd(5)
tm.assert_index_equal(rng, expected)

def test_pi_add_offset_n_gt1(self, box_transpose_fail):
def test_pi_add_offset_n_gt1(self, box5_tfail):
# GH#23215
# add offset to PeriodIndex with freq.n > 1
box, transpose = box_transpose_fail

per = pd.Period('2016-01', freq='2M')
pi = pd.PeriodIndex([per])

expected = pd.PeriodIndex(['2016-03'], freq='2M')

pi = tm.box_expected(pi, box, transpose=transpose)
expected = tm.box_expected(expected, box, transpose=transpose)
pi = tm.box_expected(pi, box5_tfail)
expected = tm.box_expected(expected, box5_tfail)

result = pi + per.freq
tm.assert_equal(result, expected)

result = per.freq + pi
tm.assert_equal(result, expected)

def test_pi_add_offset_n_gt1_not_divisible(self, box_with_period):
def test_pi_add_offset_n_gt1_not_divisible(self, box5_tfail):
# GH#23215
# PeriodIndex with freq.n > 1 add offset with offset.n % freq.n != 0
box = box_with_period

pi = pd.PeriodIndex(['2016-01'], freq='2M')
expected = pd.PeriodIndex(['2016-04'], freq='2M')

# FIXME: with transposing these tests fail
pi = tm.box_expected(pi, box, transpose=False)
expected = tm.box_expected(expected, box, transpose=False)
pi = tm.box_expected(pi, box5_tfail)
expected = tm.box_expected(expected, box5_tfail)

result = pi + to_offset('3M')
tm.assert_equal(result, expected)
Expand Down Expand Up @@ -796,16 +791,14 @@ def test_pi_add_sub_timedeltalike_freq_mismatch_monthly(self,
with pytest.raises(period.IncompatibleFrequency, match=msg):
rng -= other

def test_parr_add_sub_td64_nat(self, box_transpose_fail):
def test_parr_add_sub_td64_nat(self, box5_tfail):
# GH#23320 special handling for timedelta64("NaT")
box, transpose = box_transpose_fail

pi = pd.period_range("1994-04-01", periods=9, freq="19D")
other = np.timedelta64("NaT")
expected = pd.PeriodIndex(["NaT"] * 9, freq="19D")

obj = tm.box_expected(pi, box, transpose=transpose)
expected = tm.box_expected(expected, box, transpose=transpose)
obj = tm.box_expected(pi, box5_tfail)
expected = tm.box_expected(expected, box5_tfail)

result = obj + other
tm.assert_equal(result, expected)
Expand Down Expand Up @@ -898,10 +891,10 @@ def test_pi_ops(self):
tm.assert_index_equal(result, exp)

@pytest.mark.parametrize('ng', ["str", 1.5])
def test_pi_ops_errors(self, ng, box_with_period):
def test_pi_ops_errors(self, ng, box4):
idx = PeriodIndex(['2011-01', '2011-02', '2011-03', '2011-04'],
freq='M', name='idx')
obj = tm.box_expected(idx, box_with_period)
obj = tm.box_expected(idx, box4)

msg = r"unsupported operand type\(s\)"
with pytest.raises(TypeError, match=msg):
Expand Down
Loading