~Finish Collecting Arithmetic Tests #22350

jbrockmendel · 2018-08-14T21:56:07Z

De-duplicate fixtures and put nearly all of them in conftest

gfyoung · 2018-08-15T06:08:16Z

pandas/tests/arithmetic/conftest.py

                ids=lambda x: type(x).__name__)
 def idx(request):
    return request.param


+@pytest.fixture
+def tdser():


Perhaps td_series ? At least an underscore between td and ser would be good.

Good idea, will do.

codecov · 2018-08-15T15:06:59Z

Codecov Report

Merging #22350 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #22350   +/-   ##
=======================================
  Coverage   92.04%   92.04%           
=======================================
  Files         169      169           
  Lines       50787    50787           
=======================================
  Hits        46745    46745           
  Misses       4042     4042

Flag	Coverage Δ
#multiple	`90.45% <ø> (ø)`	⬆️
#single	`42.29% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98fb53c...25ed1e8. Read the comment docs.

jreback

comments on the fixture names. Also want to ensure that individual test files are completely obvious to what they are testing, so may want to add a level of nesting as things grow longer.

jreback · 2018-08-16T10:33:37Z

pandas/tests/arithmetic/conftest.py

+@pytest.fixture(params=[np.timedelta64(365, 'D'),
+                        pd.Timedelta(365).to_pytimedelta(),
+                        pd.Timedelta(days=365)] + _common_mismatch)
+def mismatched(request):


can you make this more descriptive

docstring doesn't get the job done?

no, prefer this to be a more descriptive name

jreback · 2018-08-16T10:33:58Z

pandas/tests/arithmetic/conftest.py

+@pytest.fixture(params=[pd.Timedelta(minutes=30).to_pytimedelta(),
+                        np.timedelta64(30, 's'),
+                        pd.Timedelta(seconds=30)] + _common_mismatch)
+def not_hourly(request):


I would add _fixture to these

can you do this

jreback · 2018-08-16T10:36:13Z

pandas/tests/arithmetic/test_numeric.py

@@ -712,6 +733,51 @@ def check(series, other):
        check(tser, 5)


+class TestUFuncCompat(object):


let's think about putting these in a separate file (test_funcs)? (may want to make numeric a subdir, its already getting long).

I'll give that some thought. Preference is to do quick turnarounds with small PRs here since there are lots of bugs to fix.

jreback · 2018-08-23T10:49:12Z

can you rebase

…ith_tests8

jbrockmendel · 2018-08-23T16:34:07Z

Rebased. After this the tests are pretty much all consolidated, so the next planned follow-up is to paramatrize/de-duplicate.

Also since#22390 just went in, upcoming PR to fix most of the outstanding xfails

…ith_tests8

gfyoung · 2018-09-03T05:07:17Z

@jbrockmendel : Have you addressed all of the comments here? I know you were waiting on @jreback to review this again, but at first glance, I can't tell if you have addressed everything he mentioned earlier.

jbrockmendel · 2018-09-03T15:42:20Z

Have you addressed all of the comments here?

I think so, yes. But since this is a big enough PR that it is hard to keep track, I made #22559 as a subset of this in the hopes it would be easier in smaller pieces.

gfyoung · 2018-09-03T17:28:13Z

I think so, yes.

@jbrockmendel : What about this one here? Just double checking! 🙂

So should we be reviewing / merging this? Or should we go after #22559 first?

jbrockmendel · 2018-09-03T17:32:49Z

What about this one here? Just double checking!

The suggestion on the fixture names was to stop fiddling for now and address in the next round. Since there were multiple comments about fixture names, its easy to lose track when one response is aimed at multiple comments.

So should we be reviewing / merging this? Or should we go after #22559 first?

Best case would be that everyone is happy here and #22559 is unnecessary. If this is going to be a slog, then I'd like to get #22559 in as this is a blocker for other work.

gfyoung · 2018-09-03T17:55:57Z

ping @jreback - could you review this again?

jreback · 2018-09-04T12:24:05Z

pandas/tests/arithmetic/conftest.py

                ids=lambda x: type(x).__name__)
-def idx(request):
+def numeric_index(request):


can you docstring

jreback · 2018-09-04T12:25:34Z

pandas/tests/arithmetic/conftest.py

+@pytest.fixture(params=[pd.Timedelta(minutes=30).to_pytimedelta(),
+                        np.timedelta64(30, 's'),
+                        pd.Timedelta(seconds=30)] + _common_mismatch)
+def not_hourly(request):


can you do this

jreback · 2018-09-04T12:26:40Z

pandas/tests/arithmetic/conftest.py

+@pytest.fixture(params=[np.timedelta64(365, 'D'),
+                        pd.Timedelta(365).to_pytimedelta(),
+                        pd.Timedelta(days=365)] + _common_mismatch)
+def mismatched(request):


no, prefer this to be a more descriptive name

jreback · 2018-09-04T12:27:03Z

pandas/tests/arithmetic/test_numeric.py

+        ts = pd.Timestamp.now()
+        df = pd.DataFrame({'x': range(5)})
+        with pytest.raises(TypeError):
+            df > ts


commented in another PR these should be in test_datetime

jreback · 2018-09-08T03:15:27Z

can you rebsae

jbrockmendel · 2018-09-09T01:36:56Z

Closing in favor of #22645.

jbrockmendel added 4 commits August 14, 2018 10:36

Move test_arithmetic.py --> tests/arithmetic/test_timedelta64.py

33ffbe8

Move most remaining arithmetic tests

e74d47a

collect fixtures in conftest

9ff309d

remove unused imports

98f871b

gfyoung added Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite labels Aug 15, 2018

gfyoung reviewed Aug 15, 2018

View reviewed changes

jbrockmendel added 2 commits August 15, 2018 07:58

rename tdser-->td_series

103723d

rename idx-->numeric_idx

ddaead0

gfyoung approved these changes Aug 15, 2018

View reviewed changes

jreback requested changes Aug 16, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Aug 16, 2018

jbrockmendel mentioned this pull request Aug 17, 2018

Fix arithmetic errors with timedelta64 dtypes #22390

Merged

3 tasks

Merge branch 'master' of https://github.com/pandas-dev/pandas into ar…

57f52cc

…ith_tests8

jbrockmendel added 2 commits August 23, 2018 14:59

Merge branch 'master' of https://github.com/pandas-dev/pandas into ar…

4ed4d47

…ith_tests8

Merge branch 'master' of https://github.com/pandas-dev/pandas into ar…

d32c65c

…ith_tests8

This was referenced Aug 30, 2018

[CLN] Dispatch (some) Frame ops to Series, avoiding _data.eval #22019

Merged

DEPR: deprecate integer add/sub with DTI/TDI/PI/Timestamp/Period #22535

Merged

Merge branch 'master' of https://github.com/pandas-dev/pandas into ar…

25ed1e8

…ith_tests8

jbrockmendel mentioned this pull request Sep 1, 2018

TST: Continue collecting arithmetic tests #22559

Merged

jreback requested changes Sep 4, 2018

View reviewed changes

jbrockmendel mentioned this pull request Sep 9, 2018

TST: Collect/Use arithmetic test fixtures #22645

Merged

jbrockmendel closed this Sep 9, 2018

jbrockmendel deleted the arith_tests8 branch September 9, 2018 01:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

~Finish Collecting Arithmetic Tests #22350

~Finish Collecting Arithmetic Tests #22350

jbrockmendel commented Aug 14, 2018

gfyoung Aug 15, 2018

jbrockmendel Aug 15, 2018

codecov bot commented Aug 15, 2018 •

edited

Loading

jreback left a comment

jreback Aug 16, 2018

jbrockmendel Aug 16, 2018

jreback Sep 4, 2018

jreback Aug 16, 2018

jreback Sep 4, 2018

jreback Aug 16, 2018

jbrockmendel Aug 16, 2018

jreback commented Aug 23, 2018

jbrockmendel commented Aug 23, 2018

gfyoung commented Sep 3, 2018

jbrockmendel commented Sep 3, 2018

gfyoung commented Sep 3, 2018

jbrockmendel commented Sep 3, 2018

gfyoung commented Sep 3, 2018

jreback Sep 4, 2018

jreback Sep 4, 2018

jreback Sep 4, 2018

jreback Sep 4, 2018

jreback commented Sep 8, 2018

jbrockmendel commented Sep 9, 2018

		@@ -712,6 +733,51 @@ def check(series, other):
		check(tser, 5)


		class TestUFuncCompat(object):

~Finish Collecting Arithmetic Tests #22350

~Finish Collecting Arithmetic Tests #22350

Conversation

jbrockmendel commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 15, 2018 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 23, 2018

jbrockmendel commented Aug 23, 2018

gfyoung commented Sep 3, 2018

jbrockmendel commented Sep 3, 2018

gfyoung commented Sep 3, 2018

jbrockmendel commented Sep 3, 2018

gfyoung commented Sep 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Sep 8, 2018

jbrockmendel commented Sep 9, 2018

codecov bot commented Aug 15, 2018 •

edited

Loading