BUG: #12624 where Panel.fillna() ignores inplace=True #12633

Xbar · 2016-03-15T13:51:01Z

Added if condition for fillna when ndim==3 (for Panel) Issue: #12624

jreback · 2016-03-15T13:59:17Z

pandas/core/generic.py

+                if inplace:
+                    new_obj = self._constructor.from_dict(result).__finalize__(self)
+                    self._update_inplace(new_obj._data)
+                    return


just do an else here

jreback · 2016-03-15T13:59:43Z

this is duped by #12630 but your soln is nicer. Needs tests.

Xbar · 2016-03-15T16:27:54Z

Added test case in test_panel.py: test_fillna
Using data provided in issue: #12624

jreback · 2016-03-15T16:31:41Z

pandas/tests/test_panel.py

+                      items=['a', 'b'], minor_axis=['x', 'y'])
+        p2 = pd.Panel([[[0, 1], [2, 1]], [[10, 11], [12, 11]]],
+                      items=['a', 'b'], minor_axis=['x', 'y'])
+        p1.fillna(method='ffill', inplace=True)


rename p2 -> expected, drop the pd.

do this in a loop with ffill, bfill

jreback · 2016-03-15T16:32:20Z

pls add a whatsnew note as well.

Xbar · 2016-03-15T22:36:00Z

Test case changes:

Add test case for Panel.fillna, method=‘bfill’
Change variable name of expected result to “expected”.
Fix discrepancy in dtype by forcing both Panel in this test case to have
dtype=np.float64

Style changes in generic.py:
Added "else" in fillna when ndim==3. Better readability.

jreback · 2016-03-15T23:47:09Z

pandas/core/generic.py

+                if inplace:
+                    new_obj = self._constructor.from_dict(result).__finalize__(self)
+                    self._update_inplace(new_obj._data)
+                    return


you don't need this return

jreback · 2016-03-15T23:48:55Z

some small changes. pls add a whatsnew note.

Xbar · 2016-03-16T00:49:32Z

Updated whatsnew.
Style changes in generic.py.
-Removed return.
-Reduced if branches when ndim==3.
-One "if" for inplace at the end of the method. Do not judge inplace twice, unless method is not implemented for some special cases.

jreback · 2016-03-16T00:53:43Z

ok looks nice. if you can squash your commits would be great. pls ping on green. (not you had some PEP errors in the prior runs which cause things to fail).
git diff master | flake8 --diff to check/fix.

jreback · 2016-03-16T00:55:40Z

https://travis-ci.org/pydata/pandas/jobs/116251314, look at the very end

jreback · 2016-03-16T00:56:21Z

pls also rebase on master: git rebase -i master (this will squash as well)

Xbar · 2016-03-16T01:16:27Z

Thanks for the reminder!
Sorry for making so many commits. I'm new to git and version control.

jreback · 2016-03-16T01:19:11Z

no, making lots of commits is fine (and good!).

At the end we just like to squash them to a single one.

codecov-io · 2016-03-16T01:51:50Z

Current coverage is `88.64%`

Merging #12633 into master will not affect coverage as of cb49b07

@@            master   #12633   diff @@
=======================================
  Files          275      275       
  Stmts       133688   133696     +8
  Branches         0        0       
  Methods          0        0       
=======================================
+ Hit         118511   118519     +8
  Partial          0        0       
  Missed       15177    15177

Review entire Coverage Diff as of cb49b07

Powered by Codecov. Updated on successful CI builds.

jreback · 2016-03-17T02:22:38Z

this needs to be squashed/rebased on master.

Xbar · 2016-03-17T02:28:28Z

I'm not sure if I'm doing it right. I'm running "git rebase -i 648b0f6" and squash all commits I made

jreback · 2016-03-17T02:35:14Z

git rebase -i master, then once squashed git push yourremote thisbranch -f

jreback · 2016-03-17T02:36:28Z

FYI, you don't want to git merge master normally, instead git pull --rebase. This is the workflow that we use here.

closes pandas-dev#12624

Xbar · 2016-03-17T02:46:20Z

I finally got the idea here. Thanks!
Now it's just 1 commit.

jreback · 2016-03-17T02:50:36Z

ok great. ping when green.

jreback · 2016-03-17T02:51:30Z

doc/source/whatsnew/v0.18.1.txt

@@ -89,46 +89,17 @@ Bug Fixes
 ~~~~~~~~~

 - Bug in ``Period`` and ``PeriodIndex`` creation raises ``KeyError`` if ``freq="Minute"`` is specified. Note that "Minute" freq is deprecated in v0.17.0, and recommended to use ``freq="T"`` instead (:issue:`11854`)
- Bug in ``Series`` construction with ``Categorical`` and ``dtype='category'`` is specified (:issue:`12574`)
-
-


in the future, don't take blank lines the whatsnew, just make your changes. I put these in because then we don't have conflicts with multiple PR's.

I see. git is telling me conflicts in whatsnew and I got confused.

Xbar · 2016-03-17T06:42:28Z

All checks passed, ready to merge.

jreback · 2016-03-17T13:54:26Z

thanks @Xbar

jreback reviewed Mar 15, 2016
View reviewed changes

jreback added Bug Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Mar 15, 2016

jreback changed the title ~~Inplace fillna for ndim==3~~ BUG: #12624 where Panel.fillna() ignores inplace=True Mar 15, 2016

jreback mentioned this pull request Mar 15, 2016

BUG: #12624 where Panel.fillna() ignores inplace=True #12630

Closed

4 tasks

jreback reviewed Mar 15, 2016
View reviewed changes

jreback added this to the 0.18.1 milestone Mar 16, 2016

Xbar force-pushed the master branch from e6c591f to 362f601 Compare March 17, 2016 02:37

BUGFIX: fillNA for panel (ndim==3) Issue: 12624

cfb3882

closes pandas-dev#12624

Xbar force-pushed the master branch from 362f601 to cfb3882 Compare March 17, 2016 02:45

jreback reviewed Mar 17, 2016
View reviewed changes

jreback closed this in ab359c1 Mar 17, 2016

jreback mentioned this pull request Mar 17, 2016

BUG: Panel.fillna() ignores inplace=True #12624

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: #12624 where Panel.fillna() ignores inplace=True #12633

BUG: #12624 where Panel.fillna() ignores inplace=True #12633

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

jreback commented Mar 15, 2016

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

jreback Mar 15, 2016

jreback commented Mar 15, 2016

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

jreback commented Mar 15, 2016

Xbar commented Mar 16, 2016

jreback commented Mar 16, 2016

jreback commented Mar 16, 2016

jreback commented Mar 16, 2016

Xbar commented Mar 16, 2016

jreback commented Mar 16, 2016

codecov-io commented Mar 16, 2016

jreback commented Mar 17, 2016

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

jreback commented Mar 17, 2016

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

jreback Mar 17, 2016

Xbar Mar 17, 2016

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

BUG: #12624 where Panel.fillna() ignores inplace=True #12633

BUG: #12624 where Panel.fillna() ignores inplace=True #12633

Conversation

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

Choose a reason for hiding this comment

jreback commented Mar 15, 2016

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

Choose a reason for hiding this comment

jreback Mar 15, 2016

Choose a reason for hiding this comment

jreback commented Mar 15, 2016

Xbar commented Mar 15, 2016

jreback Mar 15, 2016

Choose a reason for hiding this comment

jreback commented Mar 15, 2016

Xbar commented Mar 16, 2016

jreback commented Mar 16, 2016

jreback commented Mar 16, 2016

jreback commented Mar 16, 2016

Xbar commented Mar 16, 2016

jreback commented Mar 16, 2016

codecov-io commented Mar 16, 2016

Current coverage is 88.64%

jreback commented Mar 17, 2016

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

jreback commented Mar 17, 2016

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

jreback Mar 17, 2016

Choose a reason for hiding this comment

Xbar Mar 17, 2016

Choose a reason for hiding this comment

Xbar commented Mar 17, 2016

jreback commented Mar 17, 2016

Current coverage is `88.64%`