-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: None converted to NaN after groupby first and last #33462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine, can you add a whatsnew in 1.1, groupby bug fixes. merge master and ping on green.
@@ -893,7 +893,7 @@ def group_last(rank_t[:, :] out, | |||
for j in range(K): | |||
val = values[i, j] | |||
|
|||
if not checknull(val): | |||
if not checknull(val) or val is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a comment here of why we are doing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -986,7 +986,7 @@ def group_nth(rank_t[:, :] out, | |||
for j in range(K): | |||
val = values[i, j] | |||
|
|||
if not checknull(val): | |||
if not checknull(val) or val is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -94,6 +94,16 @@ def test_nth_with_na_object(index, nulls_fixture): | |||
tm.assert_frame_equal(result, expected) | |||
|
|||
|
|||
@pytest.mark.parametrize("method", ["first", "last"]) | |||
def test_first_last_with_None(method): | |||
# https://github.com/pandas-dev/pandas/issues/32800 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a comment, that we wish to preserve None in object dtypes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
thanks! |
…3462) * BUG: None converted after groupby first and last * BUG: whatsnew entry
…NaN after groupby first and last)
…groupby first and last) (#33998) Co-authored-by: JDkuba <[email protected]>
…3462) * BUG: None converted after groupby first and last * BUG: whatsnew entry
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
This PR solves #32800. In #32124
not checknull()
was introduced togroupfirst()
andgrouplast()
.None
is not caught in this conditional and it's being converted tonan
because of that.I'm a newbie and not sure whether I should add whatsnew entry or not. Should I?