Skip to content
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

Fixturize tests/frame/test_arithmetic #22736

Merged
merged 8 commits into from
Sep 26, 2018
Merged

Fixturize tests/frame/test_arithmetic #22736

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9ce1860
Fixturize tests/frame/test_arithmetic
h-vetinari Sep 17, 2018
0d8aa09
PEP8 fix
h-vetinari Sep 18, 2018
1f8a5d6
Review (jreback)
h-vetinari Sep 18, 2018
1acc6b1
PEP8
h-vetinari Sep 18, 2018
054f4e9
Merge remote-tracking branch 'upstream/master' into fixturize_frame_a…
h-vetinari Sep 19, 2018
3ee3b70
Split up test_arith_flex_frame (review jreback)
h-vetinari Sep 20, 2018
b6e14b0
Review (jreback)
h-vetinari Sep 23, 2018
b0b82a7
Review (jbrockmendel)
h-vetinari Sep 26, 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
18 changes: 10 additions & 8 deletions pandas/tests/frame/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ def mixed_float_frame():
Columns are ['A', 'B', 'C', 'D'].
"""
df = DataFrame(tm.getSeriesData())
df.A = df.A.astype('float16')
df.A = df.A.astype('float32')
df.B = df.B.astype('float32')
df.C = df.C.astype('float64')
df.C = df.C.astype('float16')
df.D = df.D.astype('float64')
return df


Expand All @@ -84,9 +85,10 @@ def mixed_float_frame2():
Columns are ['A', 'B', 'C', 'D'].
"""
df = DataFrame(tm.getSeriesData())
df.D = df.D.astype('float16')
df.D = df.D.astype('float32')
df.C = df.C.astype('float32')
df.B = df.B.astype('float64')
df.B = df.B.astype('float16')
df.D = df.D.astype('float64')
return df


Expand All @@ -99,10 +101,10 @@ def mixed_int_frame():
"""
df = DataFrame({k: v.astype(int)
for k, v in compat.iteritems(tm.getSeriesData())})
df.A = df.A.astype('uint8')
df.B = df.B.astype('int32')
df.C = df.C.astype('int64')
df.D = np.ones(len(df.D), dtype='uint64')
df.A = df.A.astype('int32')
df.B = np.ones(len(df.B), dtype='uint64')
df.C = df.C.astype('uint8')
df.D = df.C.astype('int64')
return df


Expand Down
194 changes: 79 additions & 115 deletions pandas/tests/frame/test_arithmetic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import numpy as np

from pandas.compat import range, PY3
import pandas.io.formats.printing as printing

import pandas as pd
import pandas.util.testing as tm
Expand Down Expand Up @@ -127,132 +126,97 @@ def test_df_add_flex_filled_mixed_dtypes(self):
'B': ser * 2})
tm.assert_frame_equal(result, expected)

def test_arith_flex_frame(self):
seriesd = tm.getSeriesData()
frame = pd.DataFrame(seriesd).copy()
@pytest.mark.parametrize('op', ['add', 'sub', 'mul', 'div', 'truediv',
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change these to use the fixture: all_arithmetic_operators

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

'pow', 'floordiv', 'mod'])
def test_arith_flex_frame(self, op, int_frame, mixed_int_frame,
float_frame, mixed_float_frame):

mixed_float = pd.DataFrame({'A': frame['A'].copy().astype('float32'),
'B': frame['B'].copy().astype('float32'),
'C': frame['C'].copy().astype('float16'),
'D': frame['D'].copy().astype('float64')})

intframe = pd.DataFrame({k: v.astype(int)
for k, v in seriesd.items()})
mixed_int = pd.DataFrame({'A': intframe['A'].copy().astype('int32'),
'B': np.ones(len(intframe), dtype='uint64'),
'C': intframe['C'].copy().astype('uint8'),
'D': intframe['D'].copy().astype('int64')})

# force these all to int64 to avoid platform testing issues
intframe = pd.DataFrame({c: s for c, s in intframe.items()},
dtype=np.int64)

ops = ['add', 'sub', 'mul', 'div', 'truediv', 'pow', 'floordiv', 'mod']
if not PY3:
Copy link
Contributor

Choose a reason for hiding this comment

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

you prob don't need this (when using the fixture)

aliases = {}
else:
aliases = {'div': 'truediv'}

for op in ops:
try:
alias = aliases.get(op, op)
f = getattr(operator, alias)
result = getattr(frame, op)(2 * frame)
exp = f(frame, 2 * frame)
alias = aliases.get(op, op)

f = getattr(operator, alias)
result = getattr(float_frame, op)(2 * float_frame)
exp = f(float_frame, 2 * float_frame)
tm.assert_frame_equal(result, exp)

# vs mix float
result = getattr(mixed_float_frame, op)(2 * mixed_float_frame)
exp = f(mixed_float_frame, 2 * mixed_float_frame)
tm.assert_frame_equal(result, exp)
_check_mixed_float(result, dtype=dict(C=None))

# vs mix int
Copy link
Contributor

Choose a reason for hiding this comment

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

might be more clear to move these to a separate test (from here down)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to get into orthogonal changes here. I haven't changed anything about this except removing the indentation. This goes for the test split requested below as well.

if op in ['add', 'sub', 'mul']:
result = getattr(mixed_int_frame, op)(2 + mixed_int_frame)
exp = f(mixed_int_frame, 2 + mixed_int_frame)

# no overflow in the uint
dtype = None
if op in ['sub']:
dtype = dict(B='uint64', C=None)
elif op in ['add', 'mul']:
dtype = dict(C=None)
tm.assert_frame_equal(result, exp)
_check_mixed_int(result, dtype=dtype)

# rops
r_f = lambda x, y: f(y, x)
result = getattr(float_frame, 'r' + op)(2 * float_frame)
exp = r_f(float_frame, 2 * float_frame)
tm.assert_frame_equal(result, exp)

# vs mix float
result = getattr(mixed_float_frame, op)(2 * mixed_float_frame)
exp = f(mixed_float_frame, 2 * mixed_float_frame)
tm.assert_frame_equal(result, exp)
_check_mixed_float(result, dtype=dict(C=None))

result = getattr(int_frame, op)(2 * int_frame)
exp = f(int_frame, 2 * int_frame)
tm.assert_frame_equal(result, exp)

# vs mix int
if op in ['add', 'sub', 'mul']:
result = getattr(mixed_int_frame, op)(2 + mixed_int_frame)
exp = f(mixed_int_frame, 2 + mixed_int_frame)

# no overflow in the uint
dtype = None
if op in ['sub']:
dtype = dict(B='uint64', C=None)
elif op in ['add', 'mul']:
dtype = dict(C=None)
tm.assert_frame_equal(result, exp)
_check_mixed_int(result, dtype=dtype)

# vs mix float
result = getattr(mixed_float, op)(2 * mixed_float)
exp = f(mixed_float, 2 * mixed_float)
tm.assert_frame_equal(result, exp)
_check_mixed_float(result, dtype=dict(C=None))

# vs mix int
if op in ['add', 'sub', 'mul']:
result = getattr(mixed_int, op)(2 + mixed_int)
exp = f(mixed_int, 2 + mixed_int)

# no overflow in the uint
dtype = None
if op in ['sub']:
dtype = dict(B='uint64', C=None)
elif op in ['add', 'mul']:
dtype = dict(C=None)
tm.assert_frame_equal(result, exp)
_check_mixed_int(result, dtype=dtype)

# rops
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rops were only tested within that if-block. Now they're all tested

r_f = lambda x, y: f(y, x)
result = getattr(frame, 'r' + op)(2 * frame)
exp = r_f(frame, 2 * frame)
tm.assert_frame_equal(result, exp)

# vs mix float
result = getattr(mixed_float, op)(2 * mixed_float)
exp = f(mixed_float, 2 * mixed_float)
tm.assert_frame_equal(result, exp)
_check_mixed_float(result, dtype=dict(C=None))

result = getattr(intframe, op)(2 * intframe)
exp = f(intframe, 2 * intframe)
tm.assert_frame_equal(result, exp)

# vs mix int
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, this part was tested twice. only once in cleaned-up version

if op in ['add', 'sub', 'mul']:
result = getattr(mixed_int, op)(2 + mixed_int)
exp = f(mixed_int, 2 + mixed_int)

# no overflow in the uint
dtype = None
if op in ['sub']:
dtype = dict(B='uint64', C=None)
elif op in ['add', 'mul']:
dtype = dict(C=None)
tm.assert_frame_equal(result, exp)
_check_mixed_int(result, dtype=dtype)
except:
printing.pprint_thing("Failing operation %r" % op)
raise

# ndim >= 3
ndim_5 = np.ones(frame.shape + (3, 4, 5))
msg = "Unable to coerce to Series/DataFrame"
with tm.assert_raises_regex(ValueError, msg):
f(frame, ndim_5)
Copy link
Contributor Author

@h-vetinari h-vetinari Sep 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This part (testing via the f of the op) is removed because it's testing the same thing as testing the op from the frame directly below

Copy link
Member

Choose a reason for hiding this comment

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

Can you separate out these dimension-specific tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split them out in a separate test


with tm.assert_raises_regex(ValueError, msg):
getattr(frame, op)(ndim_5)

# res_add = frame.add(frame)
# res_sub = frame.sub(frame)
# res_mul = frame.mul(frame)
# res_div = frame.div(2 * frame)

# tm.assert_frame_equal(res_add, frame + frame)
# tm.assert_frame_equal(res_sub, frame - frame)
# tm.assert_frame_equal(res_mul, frame * frame)
# tm.assert_frame_equal(res_div, frame / (2 * frame))
Copy link
Member

Choose a reason for hiding this comment

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

Did this commented-out stuff go somewhere else? Or just determined to be redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Determined to be redundant with test_arith_flex_frame, which checks all arith_ops, and much more thoroughly.


const_add = frame.add(1)
tm.assert_frame_equal(const_add, frame + 1)
# ndim >= 3
Copy link
Contributor

Choose a reason for hiding this comment

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

here and down could be in the top-section, e.g. the first test as they test all ops (or another tests is even better, with the fixture this becomes easy)

ndim_5 = np.ones(float_frame.shape + (3, 4, 5))
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment here on what this is testing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above - I did not write the test; just removed indentation

msg = "Unable to coerce to Series/DataFrame"
with tm.assert_raises_regex(ValueError, msg):
f(float_frame, ndim_5)

with tm.assert_raises_regex(ValueError, msg):
getattr(float_frame, op)(ndim_5)

const_add = float_frame.add(1)
tm.assert_frame_equal(const_add, float_frame + 1)

# corner cases
result = frame.add(frame[:0])
tm.assert_frame_equal(result, frame * np.nan)
result = float_frame.add(float_frame[:0])
tm.assert_frame_equal(result, float_frame * np.nan)

result = frame[:0].add(frame)
tm.assert_frame_equal(result, frame * np.nan)
result = float_frame[:0].add(float_frame)
tm.assert_frame_equal(result, float_frame * np.nan)
with tm.assert_raises_regex(NotImplementedError, 'fill_value'):
frame.add(frame.iloc[0], fill_value=3)
float_frame.add(float_frame.iloc[0], fill_value=3)
with tm.assert_raises_regex(NotImplementedError, 'fill_value'):
frame.add(frame.iloc[0], axis='index', fill_value=3)

def test_arith_flex_series(self):
arr = np.array([[1., 2., 3.],
[4., 5., 6.],
[7., 8., 9.]])
df = pd.DataFrame(arr, columns=['one', 'two', 'three'],
index=['a', 'b', 'c'])
float_frame.add(float_frame.iloc[0], axis='index', fill_value=3)

def test_arith_flex_series(self, simple_frame):
df = simple_frame

row = df.xs('a')
col = df['two']
Expand Down