Parametrize PeriodIndex tests #19659

jbrockmendel · 2018-02-12T16:53:15Z

closes #xxxx
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2018-02-13T04:39:59Z

Codecov Report

Merging #19659 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #19659      +/-   ##
==========================================
- Coverage   91.58%   91.58%   -0.01%     
==========================================
  Files         150      150              
  Lines       48806    48862      +56     
==========================================
+ Hits        44701    44748      +47     
- Misses       4105     4114       +9

Flag	Coverage Δ
#multiple	`89.95% <ø> (-0.01%)`	⬇️
#single	`41.75% <ø> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
pandas/io/s3.py	`72.72% <0%> (-13.64%)`	⬇️
pandas/io/packers.py	`87.31% <0%> (-1.34%)`	⬇️
pandas/core/arrays/base.py	`60% <0%> (-0.61%)`	⬇️
pandas/io/json/json.py	`92.23% <0%> (-0.37%)`	⬇️
pandas/io/common.py	`68.77% <0%> (-0.3%)`	⬇️
pandas/io/parquet.py	`71.79% <0%> (-0.28%)`	⬇️
pandas/io/parsers.py	`95.32% <0%> (-0.25%)`	⬇️
pandas/core/series.py	`94.56% <0%> (-0.06%)`	⬇️
pandas/core/frame.py	`97.16% <0%> (-0.02%)`	⬇️
pandas/core/indexes/multi.py	`95.06% <0%> (-0.01%)`	⬇️
... and 13 more

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 569bc7a...d93c11e. Read the comment docs.

jreback · 2018-02-14T00:58:28Z

pandas/tests/indexes/period/test_arithmetic.py

+        with tm.assert_raises_regex(period.IncompatibleFrequency, msg):
+            rng += other
+
+    @pytest.mark.parametrize('other', [


so I'l like to see you not repeat the parameterization on every test. you can do this in a number of ways

a fixture

make a list of the parameters (then its easy to add on a particular test)

jreback · 2018-02-15T12:33:59Z

pandas/tests/indexes/period/test_arithmetic.py

+                    pd.offsets.MonthBegin(1),
+                    pd.offsets.Minute()]
+
+not_hourly_offsets = [timedelta(minutes=30),


from here down should be fixtures, then you don't need the parameterize on each function, just pass the argument.

just pushed with this change.

jreback · 2018-02-15T12:34:46Z

pandas/tests/indexes/period/test_astype.py

+        tm.assert_numpy_array_equal(idx.astype(object).values, exp)
+        tm.assert_numpy_array_equal(idx._mpl_repr(), exp)
+
+    # TODO: de-duplicate this version (from test_ops) with the one above


when deduping (can be next pass). see if anyting can be parameterized

jbrockmendel · 2018-02-15T20:21:15Z

Multiple Travis builds have been running for almost 2hours. Is this the only PR affected?

TomAugspurger · 2018-02-16T13:10:19Z

pandas/tests/indexes/period/test_arithmetic.py

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


Could I bother you for docstrings on these? They're printed out when running pytest --fixtures, which I find really helpful in understanding what fixtures we already provide.

Just pushed a commit with these added. Let me know if they need to be fleshed out etc.

If you find the motivation, I've got a handful of small PRs that need a nudge to get over the line (i.e. haven't gotten the "ping on green", but plausibly could): #19582, #19650, #19692, #19365, #19611, #19661. Less trivial but important from a blocker-for-bugfixes standpoint is #19672.

woa didn't realize fixtures prints these out. let' fix these next for all ones that don't have them. il'll create an issue

jreback

small changes. mainly interested in the de-classing in astype. other things can also be next pass. ping on green.

jreback · 2018-02-16T18:25:40Z

pandas/tests/indexes/period/test_astype.py

+from pandas import NaT, Period, PeriodIndex, Int64Index, Index, period_range
+
+
+class TestPeriodIndexAsType(object):


can you remove the class. normally don't use a class unless you have 2 (and then I would question why we are not doing in sub-modules), unless very short

I tend to agree on the why-is-a-class-needed bit. I'd also like to keep symmetry with the test_astype modules in tests.indexes.datetimes and tests.indexes.timedeltas. In an upcoming pass I'll de-class all three at the same time.

jreback · 2018-02-16T18:27:07Z

pandas/tests/indexes/period/test_arithmetic.py

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


woa didn't realize fixtures prints these out. let' fix these next for all ones that don't have them. il'll create an issue

jreback · 2018-02-16T18:28:52Z

pandas/tests/indexes/period/test_arithmetic.py

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


jreback · 2018-02-16T18:31:07Z

pandas/tests/indexes/period/test_arithmetic.py

@@ -305,16 +300,24 @@ def test_pi_add_int(self, one):
        rng += one
        tm.assert_index_equal(rng, expected)

+    def test_pi_sub_isub_int(self, one):


prob want to doc-string one (maybe rename to slightly more informative)

jbrockmendel · 2018-02-17T03:30:07Z

ping

TomAugspurger · 2018-02-17T20:47:24Z

Thanks!

* fixup formatting * parametrize PeriodIndex tests * fixup typo * put lists of params at module level * make fixtures * docstrings for fixtures * requested docstring

jbrockmendel added 3 commits February 11, 2018 14:46

fixup formatting

4ad0d62

parametrize PeriodIndex tests

2281d0f

fixup typo

8cf154a

jreback requested changes Feb 14, 2018

View reviewed changes

jreback added Testing pandas testing functions or related to the test suite Period Period data type labels Feb 14, 2018

put lists of params at module level

7fc7c0f

jreback requested changes Feb 15, 2018

View reviewed changes

make fixtures

fa63ed1

TomAugspurger reviewed Feb 16, 2018

View reviewed changes

docstrings for fixtures

428417f

jreback requested changes Feb 16, 2018

View reviewed changes

requested docstring

d93c11e

jbrockmendel mentioned this pull request Feb 17, 2018

Fix wraparound/overflow in date_range #19740

Closed

4 tasks

TomAugspurger merged commit c8d8331 into pandas-dev:master Feb 17, 2018

jbrockmendel deleted the paramper branch February 17, 2018 21:11

jreback added this to the 0.23.0 milestone Feb 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parametrize PeriodIndex tests #19659

Parametrize PeriodIndex tests #19659

jbrockmendel commented Feb 12, 2018

codecov bot commented Feb 13, 2018 •

edited

Loading

jreback Feb 14, 2018

jreback Feb 15, 2018

jbrockmendel Feb 15, 2018

jreback Feb 15, 2018

jbrockmendel commented Feb 15, 2018

TomAugspurger Feb 16, 2018

jbrockmendel Feb 16, 2018

jreback Feb 16, 2018

jreback Feb 16, 2018

jreback left a comment

jreback Feb 16, 2018

jbrockmendel Feb 16, 2018

jreback Feb 16, 2018

jreback Feb 16, 2018

jreback Feb 16, 2018

jbrockmendel commented Feb 17, 2018

TomAugspurger commented Feb 17, 2018

		from pandas import NaT, Period, PeriodIndex, Int64Index, Index, period_range


		class TestPeriodIndexAsType(object):

Parametrize PeriodIndex tests #19659

Parametrize PeriodIndex tests #19659

Conversation

jbrockmendel commented Feb 12, 2018

codecov bot commented Feb 13, 2018 • edited Loading

Codecov Report

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

jbrockmendel commented Feb 15, 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 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

jbrockmendel commented Feb 17, 2018

TomAugspurger commented Feb 17, 2018

codecov bot commented Feb 13, 2018 •

edited

Loading