[#7292] BUG: asfreq / pct_change strange behavior #19410

minggli · 2018-01-26T12:46:18Z

closes BUG: asfreq / pct_change strange behavior #7292
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jreback · 2018-01-26T12:48:50Z

can u add tests from the original issue

minggli · 2018-01-26T12:57:54Z

Hi Jeff, can you clarify above comment? Do you mean assert pct_change(periods=5) == pct_change(periods=1, freq='5H'), as raised in the original issue?

jreback · 2018-01-26T13:10:29Z

the idea is that the original should fail w/o your fix and pass with it

codecov · 2018-01-26T13:59:24Z

Codecov Report

Merging #19410 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #19410      +/-   ##
==========================================
- Coverage   91.62%   91.62%   -0.01%     
==========================================
  Files         150      150              
  Lines       48729    48725       -4     
==========================================
- Hits        44649    44645       -4     
  Misses       4080     4080

Flag	Coverage Δ
#multiple	`89.99% <100%> (-0.01%)`	⬇️
#single	`41.74% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/generic.py	`95.91% <100%> (ø)`	⬆️
pandas/core/indexes/multi.py	`95.14% <0%> (-1.09%)`	⬇️
pandas/core/sparse/array.py	`91.48% <0%> (-0.34%)`	⬇️
pandas/core/frame.py	`97.57% <0%> (-0.05%)`	⬇️
pandas/core/sparse/series.py	`95.26% <0%> (-0.04%)`	⬇️
pandas/core/indexes/numeric.py	`97.24% <0%> (-0.04%)`	⬇️
pandas/core/indexes/timedeltas.py	`90.56% <0%> (-0.02%)`	⬇️
pandas/core/indexes/interval.py	`92.39% <0%> (-0.02%)`	⬇️
pandas/core/indexes/period.py	`92.99% <0%> (-0.02%)`	⬇️
pandas/core/indexes/base.py	`96.45% <0%> (-0.01%)`	⬇️
... and 7 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 8a7aca9...75b8942. Read the comment docs.

minggli · 2018-01-26T14:11:16Z

added test cases (frame and series) against original issue raised. @jreback

TomAugspurger · 2018-01-26T16:18:25Z

doc/source/whatsnew/v0.23.0.txt

@@ -419,6 +419,7 @@ Datetimelike
 - Bug in ``.astype()`` to non-ns timedelta units would hold the incorrect dtype (:issue:`19176`, :issue:`19223`, :issue:`12425`)
 - Bug in subtracting :class:`Series` from ``NaT`` incorrectly returning ``NaT`` (:issue:`19158`)
 - Bug in :func:`Series.truncate` which raises ``TypeError`` with a monotonic ``PeriodIndex`` (:issue:`17717`)
+- Bug in :func:`NDFrame.pct_change` produces inconsistent frames using ``periods`` and ``freq`` (:issue:`7292`)


This link to NDFrame won't work since it isn't in our API pages. Try

:func:`~DataFrame.pct_change`

It's also not clear (to me) what "inconsistent frames" means here.

added change in new commit. @TomAugspurger

minggli · 2018-01-26T20:08:20Z

@jreback @TomAugspurger let me know if this is good to go, otherwise have a good weekend. :)

jreback · 2018-01-27T16:45:04Z

pandas/tests/frame/test_timeseries.py

@@ -120,6 +122,11 @@ def test_pct_change_shift_over_nas(self):
        edf = DataFrame({'a': expected, 'b': expected})
        assert_frame_equal(chg, edf)

+    def test_pct_change_periods_freq(self):
+        rs_periods = self.tsframe.pct_change(5)


add the issue number as a comment. Can you add a few more test cases here (via parametrization if possible). include an empty frame

jreback · 2018-01-27T16:45:47Z

pandas/tests/series/test_timeseries.py

@@ -353,6 +355,11 @@ def test_pct_change_shift_over_nas(self):
        expected = Series([np.nan, 0.5, np.nan, 2.5 / 1.5 - 1, .2])
        assert_series_equal(chg, expected)

+    def test_pct_change_periods_freq(self):


same comments as above

added additional test cases and test when frame is empty.

minggli · 2018-01-30T14:26:14Z

@jreback @TomAugspurger is this good to go?

TomAugspurger

Just a few style comments. Otherwise looks good.

TomAugspurger · 2018-01-30T19:45:11Z

pandas/tests/frame/test_timeseries.py

+        rs_periods = self.tsframe.pct_change(3, fill_method='bfill')
+        assert_frame_equal(rs_freq, rs_periods)
+
+        rs_freq = \


We try to avoid \ for line continuation in pandas. We use parenthesis instead. (same comment for lines 144 and 149)

make sense :) thanks

TomAugspurger · 2018-01-30T19:45:44Z

pandas/tests/frame/test_timeseries.py

@@ -120,6 +122,36 @@ def test_pct_change_shift_over_nas(self):
        edf = DataFrame({'a': expected, 'b': expected})
        assert_frame_equal(chg, edf)

+    def test_pct_change_periods_freq(self):
+        # see issue #7292


Usually this is just # GH 7292

TomAugspurger · 2018-01-30T19:49:18Z

doc/source/whatsnew/v0.23.0.txt

@@ -419,6 +419,7 @@ Datetimelike
 - Bug in ``.astype()`` to non-ns timedelta units would hold the incorrect dtype (:issue:`19176`, :issue:`19223`, :issue:`12425`)
 - Bug in subtracting :class:`Series` from ``NaT`` incorrectly returning ``NaT`` (:issue:`19158`)
 - Bug in :func:`Series.truncate` which raises ``TypeError`` with a monotonic ``PeriodIndex`` (:issue:`17717`)
+- Bug in :func:`~DataFrame.pct_change` using ``periods`` and ``freq`` produces different sizes frames/series (:issue:`7292`)


Change "produces different sizes frames/series" to "returned different length outputs"

minggli · 2018-01-31T11:16:23Z

@TomAugspurger reviews actioned.

jreback · 2018-01-31T11:34:23Z

thanks @minggli

minggli · 2018-01-31T11:37:19Z

happy to contribute! 🚀

…ev#19410)

minggli added 4 commits January 26, 2018 12:16

reindex resulting NDFrame after percentage calculation

e7f4f4e

update pct_change test case with freq shift

d0baf8f

update pct_change test case with freq shift

63701a3

whatsnew entry for issue:7292

8cfb77a

minggli added 2 commits January 26, 2018 14:08

add new test case for consistent pct_change output

e594e04

add new test case for consistent pct_change output

dc9d64e

TomAugspurger reviewed Jan 26, 2018

View reviewed changes

rephase bugfix documentation

22e0c34

jreback added Datetime Datetime data dtype Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Jan 27, 2018

jreback requested changes Jan 27, 2018

View reviewed changes

minggli added 2 commits January 27, 2018 20:40

additional test cases test_pct_change_periods_freq

a1d804f

additional test cases test_pct_change_periods_freq

106bdf4

minggli closed this Jan 27, 2018

minggli reopened this Jan 27, 2018

empty containre without numpy

b6377df

minggli closed this Jan 28, 2018

minggli reopened this Jan 28, 2018

minggli closed this Jan 29, 2018

minggli reopened this Jan 29, 2018

TomAugspurger reviewed Jan 30, 2018

View reviewed changes

minggli added 2 commits January 30, 2018 20:03

rephase documentation whatsnew

b08d4c8

remove slash for line continuation

27aee76

change comment to GH 7292

75b8942

jreback added this to the 0.23.0 milestone Jan 31, 2018

jreback approved these changes Jan 31, 2018

View reviewed changes

jreback merged commit 1bd7b3a into pandas-dev:master Jan 31, 2018

minggli deleted the bugfixs/7292 branch January 31, 2018 11:36

minggli mentioned this pull request Feb 25, 2018

parameterize test_pct_change_periods_freq #19897

Merged

4 tasks

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018

[pandas-dev#7292] BUG: asfreq / pct_change strange behavior (pandas-d…

1c64d7d

…ev#19410)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#7292] BUG: asfreq / pct_change strange behavior #19410

[#7292] BUG: asfreq / pct_change strange behavior #19410

minggli commented Jan 26, 2018 •

edited

Loading

jreback commented Jan 26, 2018

minggli commented Jan 26, 2018 •

edited

Loading

jreback commented Jan 26, 2018

codecov bot commented Jan 26, 2018 •

edited

Loading

minggli commented Jan 26, 2018 •

edited

Loading

TomAugspurger Jan 26, 2018

minggli Jan 26, 2018 •

edited

Loading

minggli commented Jan 26, 2018 •

edited

Loading

jreback Jan 27, 2018

jreback Jan 27, 2018

minggli Jan 27, 2018

minggli commented Jan 30, 2018

TomAugspurger left a comment

TomAugspurger Jan 30, 2018

minggli Jan 30, 2018

TomAugspurger Jan 30, 2018

minggli Jan 30, 2018

TomAugspurger Jan 30, 2018

minggli Jan 30, 2018

minggli commented Jan 31, 2018

jreback commented Jan 31, 2018

minggli commented Jan 31, 2018

[#7292] BUG: asfreq / pct_change strange behavior #19410

[#7292] BUG: asfreq / pct_change strange behavior #19410

Conversation

minggli commented Jan 26, 2018 • edited Loading

jreback commented Jan 26, 2018

minggli commented Jan 26, 2018 • edited Loading

jreback commented Jan 26, 2018

codecov bot commented Jan 26, 2018 • edited Loading

Codecov Report

minggli commented Jan 26, 2018 • edited Loading

Choose a reason for hiding this comment

minggli Jan 26, 2018 • edited Loading

Choose a reason for hiding this comment

minggli commented Jan 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minggli commented Jan 30, 2018

TomAugspurger 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

minggli commented Jan 31, 2018

jreback commented Jan 31, 2018

minggli commented Jan 31, 2018

minggli commented Jan 26, 2018 •

edited

Loading

minggli commented Jan 26, 2018 •

edited

Loading

codecov bot commented Jan 26, 2018 •

edited

Loading

minggli commented Jan 26, 2018 •

edited

Loading

minggli Jan 26, 2018 •

edited

Loading

minggli commented Jan 26, 2018 •

edited

Loading