BUG : Allow axis = 'columns' with limit and no method to work on fillna. #42856
Conversation
GYvan
changed the title
GYvan
changed the title
jreback
added
Bug
Missing-data
GYvan
changed the title
GYvan
changed the title
GYvan
changed the title
| @@ -438,6 +438,22 @@ def test_fillna_inplace(self): | |||
| df.fillna(method="ffill", inplace=True) | |||
| tm.assert_frame_equal(df, expected) | |||
|
|
|||
| df = DataFrame( | |||
| new_data = self._mgr.fillna( | ||
| value=value, limit=limit, inplace=inplace, downcast=downcast | ||
| ) | ||
| if not self._mgr.is_single_block and axis == 1: |
There was a problem hiding this comment.
isn't this the same as L6254
There was a problem hiding this comment.
Yes, the code is somehow the same, with minor adjustments. I basically used it to make transposes as the ones done when the method is not None. This is because when the method is valid, both axis = 1 and axis = 0 works correctly with the given limit for Fillna due to the transposition at L6254. Therefore, I tried to do the same kind of transposing so that axis = 1 and axis = 0 would work accordingly for the given value and limit.
There was a problem hiding this comment.
ok any possibiltiy of combing those cases into a similar code block?
There was a problem hiding this comment.
Hi @jreback I tried to combine these cases into a similar code block and this is the code that I came up with for a separate top-level if-else block in fillna:
if not self._mgr.is_single_block and axis == 1:
if value is None or not is_list_like(value):
if inplace and value is None:
raise NotImplementedError()
result = self.T.fillna(method=method, value=value, limit=limit).T
# need to downcast here because of all of the transposes
result._mgr = result._mgr.downcast()
if value is None:
return result
new_data = result
I also added the condition for x==0 for the remaining of the elif not is_list_like(value) so that it won't interfere with the above changes with x == 1.
However, due to the additional code block: the code structure is slightly modified because before, the code for fillna was clearly split between the case for when value is None and the case for when value is not None, but this additional code block changes that. This new code block means that there are now two places where we would be "branching" on whether a value is None or not: Firstly, in the new code block, and secondly, in the original if-else block. Therefore, I think the new change might make it more difficult to follow the code compared to the original PR where the change was only made to the code that affects fillna with axis = 1 and value is not None.
| new_data = self._mgr.fillna( | ||
| value=value, limit=limit, inplace=inplace, downcast=downcast | ||
| ) | ||
| if not self._mgr.is_single_block and axis == 1: |
There was a problem hiding this comment.
ok any possibiltiy of combing those cases into a similar code block?
|
lgtm. @mzeitlin11 or @phofl if you'd take a quick look here. |
|
thanks @GYvan |
…na. (pandas-dev#42856) * solves issue pandas-dev#40989 by raising NotImplementedError and modifying docs * Testing the NotImplementedError created to solve issue pandas-dev#40989 * pre-commit error fixed * added modified generic.py * added modified test_fillna.py * reverted changes for test_frame.py * added another test * added into bug fixes in whatsnew
Added an if-condition for when axis = 1 so as to transpose the result in pandas/tests/frame/methods/test_fillna.py, and created a test for this change. The test also tests for
inplace =True. This solves issue #40989