Skip to content

Conversation

@ueshin
Copy link
Collaborator

@ueshin ueshin commented Oct 6, 2020

Fixes the input check in groupby.

@ueshin ueshin requested review from HyukjinKwon and itholic October 6, 2020 01:20
)

# TODO: a pandas bug?
# expected = pdf.groupby((10, "a"))[(20, "c")].sum().sort_index()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit of nit: one more space here in the comment 😅

Copy link
Contributor

@itholic itholic Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, pdf.groupby((10, "a"))[[(20, "c")]].sum() seems working.

>>> pdf
  10      20 30
   a  b    c  d
0  1  4  4.0  a
1  2  2  2.0  b
3  6  7  7.0  c
5  4  3  3.0  d
6  4  3  NaN  e
8  6  1  1.0  f
9  4  1  1.0  g
9  3  1  1.0  h
9  7  2  2.0  t

>>> pdf.groupby((10, "a"))[[(20, "c")]].sum()
          20
           c
(10, a)
1        4.0
2        2.0
3        1.0
4        4.0
6        8.0
7        2.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, Sorry. nvm above. It's different from pdf.groupby((10, "a"))[(20, "c")].sum() anyway.

Comment on lines -2348 to -2351
self.assertRaises(
KeyError,
lambda: kdf.groupby(("B", "class"))[("A", "name")].get_group(["bird", "mammal"]),
)
Copy link
Contributor

@itholic itholic Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed because this is duplicated test with below ?

@itholic
Copy link
Contributor

itholic commented Oct 7, 2020

LGTM. Anyway I think it's good to merge regardless of two nit comments.

@HyukjinKwon HyukjinKwon merged commit 7d74e3f into databricks:master Oct 7, 2020
@ueshin ueshin deleted the check_non-string_names/groupby branch October 7, 2020 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants