BUG: Empty PeriodIndex issues #13079

max-sixty · 2016-05-04T02:52:47Z

closes BUG: Period resample with length=0 doesn't set freq #13067 (existing PR)
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

sinhrks · 2016-05-04T04:34:11Z

pandas/tseries/tests/test_period.py

+    def test_shallow_copy_empty(self):
+
+        # GH13067
+        idx = PeriodIndex([], freq='M')


can u also test _simple_new adding name attribute?

The test above does that, no?

above doesn't check empty case. Though _shallow_copy internally uses _simple_new, I meant to guarantee _simple_new API.

max-sixty · 2016-05-04T06:03:38Z

pandas/tseries/period.py

+            try:  # assume values / ints
+                values = np.array(values, copy=False, dtype='int64')
+            except TypeError: # objects / Periods
+                return PeriodIndex(values, name=name, freq=freq, **kwargs)


@jreback Is this the best way to do this? Previously it was creating float64 arrays.

that would probably work. IIUC, this is a non ndarray, that could be ints, or objects (meaning Periods, or coercable to).

have a look at com.is_period_arraylike as well. Though that actually traverses.

com.is_period_arraylike didn't work, so we're back to the try except

jreback · 2016-05-04T12:19:17Z

doc/source/whatsnew/v0.18.2.txt

@@ -74,3 +74,7 @@ Performance Improvements

 Bug Fixes
 ~~~~~~~~~
+
+- Bug in ``PeriodIndex.resample`` not changing ``freq`` when empty


explain what changing is.

also say: .resample(..) with a PeriodIndex

max-sixty · 2016-05-04T18:10:37Z

@jreback @sinhrks:
Any idea what this might be? It only happens on 2.7 and so I would need to rebuild to step through. No prob if nothing comes to mind though:

======================================================================
ERROR: test_resample_empty (pandas.tseries.tests.test_resample.TestPeriodIndex)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/pydata/pandas/pandas/tseries/tests/test_resample.py", line 2054, in test_resample_empty
    result = getattr(s.resample(freq), method)()
  File "/home/travis/build/pydata/pandas/pandas/tseries/resample.py", line 511, in f
    return self._downsample(_method)
  File "/home/travis/build/pydata/pandas/pandas/tseries/resample.py", line 805, in _downsample
    return self._groupby_and_aggregate(how, grouper=grouper)
  File "/home/travis/build/pydata/pandas/pandas/tseries/resample.py", line 379, in _groupby_and_aggregate
    result = grouped.apply(how, *args, **kwargs)
  File "/home/travis/build/pydata/pandas/pandas/core/groupby.py", line 645, in apply
    @wraps(func)
  File "/home/travis/miniconda/envs/pandas/lib/python2.7/functools.py", line 33, in update_wrapper
    setattr(wrapper, attr, getattr(wrapped, attr))
AttributeError: 'str' object has no attribute '__module__'
----------------------------------------------------------------------
Ran 10105 tests in 537.776s
FAILED (SKIP=177, errors=1)

jreback · 2016-05-04T18:11:54Z

I remember seeing those. I think an attribute is being passed which isn't defined for that object
e.g. nunique for a DataFrame will cause this.

jreback · 2016-05-05T16:10:47Z

doc/source/whatsnew/v0.18.2.txt

@@ -74,3 +74,8 @@ Performance Improvements

 Bug Fixes
 ~~~~~~~~~
+
+- Bug in ``.resample(..)`` with a ``PeriodIndex`` not changing its ``freq``


doesn't make sense; the bug is returning an incorrect freq when empty, right?

sinhrks · 2016-05-07T09:51:03Z

pandas/tseries/period.py

-            return PeriodIndex(values, name=name, freq=freq, **kwargs)
+            try:  # assume values / ints
+                values = np.array(values, copy=False, dtype='int64')
+            except TypeError:  # objects / Periods


This looks to change the current behavior below.

pd.PeriodIndex._simple_new(np.array([pd.Period('2011-01', freq='M')])) # PeriodIndex(['2011-01'], dtype='int64', freq='M')

I think that is correct though, any reason we don't want that?

@MaximilianR as an aside pls run asv on --bench period to verify no changes (under py3) in perf

This is ugly and I would love a better way of doing it.
I do think the outcomes are correct though - @sinhrks do you disagree?
Open to nice implementation though

I just wondering whether the following behavior (on current master) are the same. Also is no inconsistencies between list and ndarray.

pd.PeriodIndex._simple_new([pd.Period('2011-01', freq='M')]) # PeriodIndex(['2011-01'], dtype='int64', freq='M') pd.PeriodIndex._simple_new(np.array([pd.Period('2011-01', freq='M')])) # PeriodIndex(['2011-01'], dtype='int64', freq='M')

pd.PeriodIndex._simple_new([1.1], freq='M') # ValueError: Ordinal must be an integer pd.PeriodIndex._simple_new(np.array([1.1]), freq='M') # ValueError: Ordinal must be an integer

@sinhrks not sure I understand, those seem right.

I think @sinhrks is asking whether the new implementation is consistent with those examples, from master. I'll check

It wasn't working well. Changed & tested.

jreback · 2016-05-14T16:39:55Z

want to rebase/update

max-sixty · 2016-05-18T22:53:28Z

is that #13212 ?

Yup!

sinhrks · 2016-05-19T20:39:49Z

pandas/tseries/tests/test_resample.py

+
+                result = getattr(s.resample(freq), method)()
+                expected = s
+                assert_index_equal(result.index, expected_index)


can u explicitly compare .freq also, as assert_index_equal doesn't (i'll submit another PR once this is fixed).

max-sixty · 2016-05-20T16:22:59Z

Should I remove this test now @jreback? I think it's covered by the Base tests now https://github.com/pydata/pandas/blob/master/pandas/tseries/tests/test_resample.py#L1411

jreback · 2016-05-20T16:30:42Z

yes (just inspect it to make sure that we are covering the bases)

max-sixty · 2016-05-20T16:40:12Z

OK, I moved a couple around to ensure we weren't losing anything

jreback · 2016-05-20T16:49:03Z

pandas/tseries/tests/test_resample.py

@@ -572,12 +573,18 @@ def test_agg_consistency(self):
        result = r.agg({'r1': 'mean', 'r2': 'sum'})
        assert_frame_equal(result, expected)

+    def test_raises_on_non_datetimelike_index(self):


can you put this in Base? (e.g. its a generic test)

done. I had thought generic was to run tests across a number of types, while this doesn't depend on the index type.

max-sixty · 2016-05-21T04:03:36Z

@jreback @sinhrks Green!

jreback · 2016-05-21T14:15:00Z

thanks @MaximilianR @sinhrks this is great8!

small issue. I am not convinced that we are testing IncompatibleFrequency fully, IOW we may be just testing for ValueError. But that's a minor point & can be done as a followup.

sinhrks · 2016-05-21T14:53:09Z

thanks @MaximilianR :)

max-sixty force-pushed the period_resample_0 branch from 8ba87d0 to db8b3ad Compare May 4, 2016 02:54

sinhrks reviewed May 4, 2016
View reviewed changes

sinhrks added Bug Period Period data type Resample resample method labels May 4, 2016

sinhrks added this to the 0.18.2 milestone May 4, 2016

max-sixty force-pushed the period_resample_0 branch from db8b3ad to 724c587 Compare May 4, 2016 06:02

max-sixty reviewed May 4, 2016
View reviewed changes

max-sixty force-pushed the period_resample_0 branch 2 times, most recently from a486745 to 2cac538 Compare May 4, 2016 11:56

jreback reviewed May 4, 2016
View reviewed changes

max-sixty force-pushed the period_resample_0 branch 3 times, most recently from c23aafb to cca6cd3 Compare May 4, 2016 16:27

jreback reviewed May 5, 2016
View reviewed changes

jreback mentioned this pull request May 5, 2016

BUG: Period resample with length=0 doesn't set freq #13067

Closed

3 tasks

sinhrks reviewed May 7, 2016
View reviewed changes

max-sixty force-pushed the period_resample_0 branch 2 times, most recently from 19d5178 to 5982e21 Compare May 19, 2016 17:58

sinhrks reviewed May 19, 2016
View reviewed changes

max-sixty mentioned this pull request May 19, 2016

PeriodIndex with float input inconsistency #13232

Closed

max-sixty force-pushed the period_resample_0 branch 4 times, most recently from 0672f28 to c65e83e Compare May 20, 2016 16:16

max-sixty force-pushed the period_resample_0 branch from c65e83e to 5bd2f40 Compare May 20, 2016 16:29

max-sixty force-pushed the period_resample_0 branch 2 times, most recently from 81cb7a3 to 1e539df Compare May 20, 2016 16:39

jreback reviewed May 20, 2016
View reviewed changes

max-sixty force-pushed the period_resample_0 branch from 1e539df to 4a5fa12 Compare May 20, 2016 17:50

sinhrks mentioned this pull request May 20, 2016

TST: test_assert_index check index attributes #13242

Closed

5 tasks

empty PeriodIndex issues

8c7b9db

max-sixty force-pushed the period_resample_0 branch from 4a5fa12 to 8c7b9db Compare May 21, 2016 00:14

sinhrks mentioned this pull request May 21, 2016

TST/ERR: Add Period ops tests / fix error message #13250

Closed

3 tasks

jreback closed this in d2b5819 May 21, 2016

jreback mentioned this pull request May 21, 2016

TST: assert that IncompatibleFrequency is raise (rather than ValueError) #13251

Closed

max-sixty deleted the period_resample_0 branch May 21, 2016 15:17

max-sixty mentioned this pull request May 21, 2016

BUG: resample with empty DatetimeIndex/TimedeltaIndex buggy #12886

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Empty PeriodIndex issues #13079

BUG: Empty PeriodIndex issues #13079

max-sixty commented May 4, 2016 •

edited by jreback

Loading

sinhrks May 4, 2016

max-sixty May 4, 2016

sinhrks May 4, 2016

max-sixty May 4, 2016

jreback May 4, 2016

max-sixty May 4, 2016

jreback May 4, 2016

jreback May 4, 2016

max-sixty commented May 4, 2016

jreback commented May 4, 2016

jreback May 5, 2016

sinhrks May 7, 2016

jreback May 7, 2016

max-sixty May 7, 2016

sinhrks May 7, 2016 •

edited

Loading

jreback May 8, 2016

max-sixty May 8, 2016

max-sixty May 17, 2016

jreback commented May 14, 2016

max-sixty commented May 18, 2016 •

edited

Loading

sinhrks May 19, 2016

max-sixty commented May 20, 2016

jreback commented May 20, 2016

max-sixty commented May 20, 2016

jreback May 20, 2016

max-sixty May 20, 2016

max-sixty commented May 21, 2016

jreback commented May 21, 2016

sinhrks commented May 21, 2016

BUG: Empty PeriodIndex issues #13079

BUG: Empty PeriodIndex issues #13079

Conversation

max-sixty commented May 4, 2016 • edited by jreback Loading

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

max-sixty commented May 4, 2016

jreback commented May 4, 2016

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

sinhrks May 7, 2016 • edited Loading

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 May 14, 2016

max-sixty commented May 18, 2016 • edited Loading

Choose a reason for hiding this comment

max-sixty commented May 20, 2016

jreback commented May 20, 2016

max-sixty commented May 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-sixty commented May 21, 2016

jreback commented May 21, 2016

sinhrks commented May 21, 2016

max-sixty commented May 4, 2016 •

edited by jreback

Loading

sinhrks May 7, 2016 •

edited

Loading

max-sixty commented May 18, 2016 •

edited

Loading