BUG: Inconsistent return type for downsampling on resample of empty DataFrame #15093

discort · 2017-01-09T21:58:41Z

closes Inconsistent return type for downsampling on resample of empty DataFrame #14962
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

jreback · 2017-01-09T22:13:17Z

pandas/tseries/tests/test_resample.py

+                           index=pd.date_range('1/1/2000', periods=100, freq="M"))
+        df2 = df1[df1.a < 0]
+        result = df2.resample("Q").size()
+        assertIsInstance(result, pd.Series)


construct the anticipated series and use assert_series_equal instead

jreback · 2017-01-09T22:14:11Z

update this test as well (IOW add size)

def test_resample_empty_dataframe(self):
        # GH13212
        index = self.create_series().index[:0]
        f = DataFrame(index=index)

        for freq in ['M', 'D', 'H']:
            # count retains dimensions too
            methods = downsample_methods + ['count']
            for method in methods:
                result = getattr(f.resample(freq), method)()

                expected = f.copy()
                expected.index = f.index._shallow_copy(freq=freq)
                assert_index_equal(result.index, expected.index)
                self.assertEqual(result.index.freq, expected.index.freq)
                assert_frame_equal(result, expected, check_dtype=False)

jreback · 2017-01-09T22:15:02Z

pandas/tseries/resample.py

@@ -686,6 +686,10 @@ def _downsample(self, how, **kwargs):
        if not len(ax):
            # reset to the new freq
            obj = obj.copy()
+            if how == "size" and isinstance(obj, pd.DataFrame):


this is not a good way of fixing. you will have to trace thru the .size implementation.

@jreback the size method of resampler of the empty dataframe must always return the empty series for each frequency, doesn't it?

the easiest I think is to pass the result thru .wrap_result(..)

jreback

needs a whatsnew note, but more importantly needs work on the impl.

codecov-io · 2017-01-10T02:28:39Z

Codecov Report

Merging #15093 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15093      +/-   ##
==========================================
- Coverage   90.95%   90.92%   -0.03%     
==========================================
  Files         161      161              
  Lines       49276    49282       +6     
==========================================
- Hits        44817    44811       -6     
- Misses       4459     4471      +12

Flag	Coverage Δ
#multiple	`88.68% <100%> (-0.03%)`	⬇️
#single	`40.18% <42.85%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/resample.py	`96.13% <100%> (+0.03%)`	⬆️
pandas/plotting/_converter.py	`63.23% <0%> (-1.82%)`	⬇️

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 344cec7...44f52be. Read the comment docs.

jreback · 2017-01-14T18:18:15Z

doc/source/whatsnew/v0.19.0.txt

@@ -1563,3 +1563,4 @@ Bug Fixes
 - ``PeriodIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
 - Bug in ``df.groupby`` where ``.median()`` returns arbitrary values if grouped dataframe contains empty bins (:issue:`13629`)
 - Bug in ``Index.copy()`` where ``name`` parameter was ignored (:issue:`14302`)


moves to 0.20.0

give a user friendly function here, _downsample is internal.

jreback · 2017-01-14T18:19:54Z

pandas/tseries/resample.py

@@ -756,6 +756,15 @@ def _wrap_result(self, result):
        # convert if needed
        if self.kind == 'period' and not isinstance(result.index, PeriodIndex):
            result.index = result.index.to_period(self.freq)
+
+        # Make consistent type of result. GH14962


is all this necessary? should be simpler

jreback · 2017-01-14T18:20:12Z

pandas/tseries/tests/test_resample.py

@@ -730,7 +730,7 @@ def test_resample_empty_series(self):
                    assert_series_equal(result, expected, check_dtype=False,
                                        check_names=False)
                    # this assert will break when fixed
-                    self.assertTrue(result.name is None)
+                    # self.assertTrue(result.name is None)


what are you doing here?

jreback · 2017-01-14T18:20:36Z

pandas/tseries/tests/test_resample.py

            for method in methods:
                result = getattr(f.resample(freq), method)()

-                expected = f.copy()
+                expected = pd.Series([])


this is not correct, it doesn't preserve things like name.

@jreback. Could you please explain the name you are talking about? The name of the Series? How could exist it if the Series created after DataFrame resampling?

right, you are resampling an empty dataframe here, so the result must be an empty dataframe with the new freq.

its not a Series at all.

jreback · 2017-01-14T18:20:56Z

pandas/tseries/tests/test_resample.py

@@ -806,6 +806,16 @@ def test_resample_loffset_arg_type(self):
                assert_frame_equal(result_agg, expected)
                assert_frame_equal(result_how, expected)

+    def test_resample_empty_dataframe_with_size(self):


this is repeating the above tests

discort · 2017-01-23T23:58:08Z

@jreback could you please take a look, that's solution is okay or not?

jreback · 2017-01-24T14:42:36Z

doc/source/whatsnew/v0.19.0.txt

@@ -1562,4 +1562,4 @@ Bug Fixes
 - Bugs in ``stack``, ``get_dummies``, ``make_axis_dummies`` which don't preserve categorical dtypes in (multi)indexes (:issue:`13854`)
 - ``PeriodIndex`` can now accept ``list`` and ``array`` which contains ``pd.NaT`` (:issue:`13430`)
 - Bug in ``df.groupby`` where ``.median()`` returns arbitrary values if grouped dataframe contains empty bins (:issue:`13629`)
- Bug in ``Index.copy()`` where ``name`` parameter was ignored (:issue:`14302`)


revert this file

jreback · 2017-01-24T14:43:18Z

doc/source/whatsnew/v0.20.0.txt

-
-
- Bug in ``DataFrame.boxplot`` where ``fontsize`` was not applied to the tick labels on both axes (:issue:`15108`)
+- Bug in ``resample().size()``. Inconsistent return type on resample of empty DataFrame (:issue:`14962`)


just move this in the bug fix section up higher, that way you won't have conflicts

jreback · 2017-01-24T14:43:45Z

pandas/tseries/resample.py

@@ -567,8 +579,13 @@ def f(self, _method=method, *args, **kwargs):
 # groupby & aggregate methods
 for method in ['count', 'size']:

-    def f(self, _method=method):
-        return self._downsample(_method)
+    if method == "size":


I don't think any of this is necessary, _wrap_results can handle all of this

jreback · 2017-01-24T14:43:50Z

pandas/tseries/resample.py

+
+        # Make consistent type of result. GH14962
+        if not len(self.ax) and isinstance(result, ABCDataFrame) and \
+                isinstance(result.index, (DatetimeIndex, TimedeltaIndex)):


this is not necessary

you can do this logic in _wrap_results, but I again don't think this is ncessary, simply define the size method as it 'seems' special and needs extra handling.

jreback · 2017-01-24T14:45:06Z

pandas/tseries/tests/test_resample.py

+        for freq in ['M', 'D', 'H']:
+            result = f.resample(freq).size()
+
+            if isinstance(result.index, PeriodIndex) and freq == 'H':


why is this any different?

jreback · 2017-02-16T17:51:35Z

can you rebase / update

jreback · 2017-04-03T15:19:11Z

can you rebase / update

discort · 2017-04-05T10:25:30Z

@jreback

jreback · 2017-04-20T10:59:03Z

@discort sorry been MIA on this one. can you rebase.

jreback · 2017-04-20T11:00:16Z

pandas/tests/tseries/test_resample.py

                expected.index = f.index._shallow_copy(freq=freq)
                assert_index_equal(result.index, expected.index)
                self.assertEqual(result.index.freq, expected.index.freq)
-                assert_frame_equal(result, expected, check_dtype=False)
+                assert_equal(result, expected, check_dtype=False)


we don't use assert_equal

jreback · 2017-05-13T21:38:10Z

can you rebase

jreback · 2017-06-10T19:05:52Z

can you rebase and update?

discort · 2017-06-13T06:54:54Z

@jreback

jreback

lgtm minor comments. ping on green.

jreback · 2017-06-13T10:03:56Z

pandas/tests/test_resample.py

                expected.index = f.index._shallow_copy(freq=freq)
                assert_index_equal(result.index, expected.index)
                assert result.index.freq == expected.index.freq
-                assert_frame_equal(result, expected, check_dtype=False)
+                assert_type_equal(result, expected, check_dtype=False)


you can use assert_almost_equal here (it dispatches) rather than rolling your own

jreback · 2017-06-13T10:09:59Z

pandas/core/resample.py

@@ -549,6 +550,14 @@ def var(self, ddof=1, *args, **kwargs):
        nv.validate_resampler_func('var', args, kwargs)
        return self._downsample('var', ddof=ddof)

+    @Appender(GroupBy.size.__doc__)
+    def size(self):
+        # It 'seems' special and needs extra handling. GH14962


better comment here. say its a special case as higher level does returns a copy of 0-len objects.

jreback · 2017-06-13T10:11:40Z

doc/source/whatsnew/v0.21.0.txt

@@ -112,6 +112,7 @@ Plotting

 Groupby/Resample/Rolling
 ^^^^^^^^^^^^^^^^^^^^^^^^
+- Bug in ``resample().size()``. Inconsistent return type on resample of empty DataFrame (:issue:`14962`)


Bug in DataFrame.resample().size() where an empty DataFrame did not return a Series.

discort · 2017-06-13T14:45:01Z

@jreback

jreback · 2017-06-13T23:16:07Z

thanks!

jreback changed the title ~~BUG #14962~~ BUG: Inconsistent return type for downsampling on resample of empty DataFrame Jan 9, 2017

jreback reviewed Jan 9, 2017

View reviewed changes

jreback added Bug Resample resample method Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Jan 9, 2017

jreback requested changes Jan 9, 2017

View reviewed changes

discort force-pushed the fix_14962_bug branch from 46e9282 to c41d339 Compare January 11, 2017 22:19

jreback requested changes Jan 14, 2017

View reviewed changes

discort force-pushed the fix_14962_bug branch from c41d339 to 427234f Compare January 23, 2017 19:54

jreback requested changes Jan 24, 2017

View reviewed changes

discort force-pushed the fix_14962_bug branch from 427234f to 986f7c2 Compare January 25, 2017 07:50

discort force-pushed the fix_14962_bug branch from 986f7c2 to 766a77f Compare February 13, 2017 20:28

discort force-pushed the fix_14962_bug branch from 766a77f to e5ab684 Compare February 16, 2017 18:23

discort force-pushed the fix_14962_bug branch from e5ab684 to 202cd4e Compare April 5, 2017 06:42

jreback reviewed Apr 20, 2017

View reviewed changes

discort force-pushed the fix_14962_bug branch 2 times, most recently from a98536a to f73a3da Compare June 12, 2017 20:08

jreback approved these changes Jun 13, 2017

View reviewed changes

jreback added this to the 0.21.0 milestone Jun 13, 2017

discort added 6 commits June 13, 2017 15:22

BUG pandas-dev#14962

95d2008

BUG: added series type to wrap_result for empty DataFrame

8f7b437

added explicit 'size' method and defined logic there

37ba820

fixed lint warnings

43b2613

Updated tests for resampling of empty dataframes

c0e06b8

Fixed Jeff's comments

44f52be

discort force-pushed the fix_14962_bug branch from f73a3da to 44f52be Compare June 13, 2017 12:48

jreback merged commit d02ef6f into pandas-dev:master Jun 13, 2017



		- Bug in ``DataFrame.boxplot`` where ``fontsize`` was not applied to the tick labels on both axes (:issue:`15108`)
		- Bug in ``resample().size()``. Inconsistent return type on resample of empty DataFrame (:issue:`14962`)

BUG: Inconsistent return type for downsampling on resample of empty DataFrame #15093

BUG: Inconsistent return type for downsampling on resample of empty DataFrame #15093

Conversation

discort commented Jan 9, 2017 • edited Loading

Choose a reason for hiding this comment

jreback commented Jan 9, 2017

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

codecov-io commented Jan 10, 2017 • edited by codecov bot 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

discort Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

discort commented Jan 23, 2017

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 Feb 16, 2017

jreback commented Apr 3, 2017

discort commented Apr 5, 2017

jreback commented Apr 20, 2017

Choose a reason for hiding this comment

jreback commented May 13, 2017

jreback commented Jun 10, 2017

discort commented Jun 13, 2017

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

discort commented Jun 13, 2017

jreback commented Jun 13, 2017

discort commented Jan 9, 2017 •

edited

Loading

codecov-io commented Jan 10, 2017 •

edited by codecov bot

Loading

discort Jan 16, 2017 •

edited

Loading