Skip to content

STYLE: fix pylint unnecessary-comprehension warnings #49674

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

Merged
merged 3 commits into from
Nov 13, 2022

Conversation

Moisan
Copy link
Contributor

@Moisan Moisan commented Nov 12, 2022

Related to #48855

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm, ping on green

Comment on lines 949 to 953
# calling `dict` on a DataFrameGroupBy leads to a TypeError,
# we need to use a dictionary comprehension here
groups = {
key: gp for key, gp in grouped
} # pylint: disable=unnecessary-comprehension
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related question: is it intentional that dict(df.groupby(["k1", "k2"])) leads to a TypeError while it's possible to iterate on it as if it was a dictionary?

Copy link
Member

Choose a reason for hiding this comment

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

Not exactly intentional, but I don't think there is a good way to make it work. The dict constructor either takes a mapping or an iterable. A groupby object has all the data for a mapping, but doesn't support accessing like a map. We can however use the fact that dict takes an iterable - you should be able to use dict(grouped.items()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dict(grouped.items()) leads to

AttributeError: 'DataFrameGroupBy' object has no attribute 'items'

Copy link
Member

@rhshadrach rhshadrach Nov 13, 2022

Choose a reason for hiding this comment

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

Ah, indeed! A couple of issues with my previous comment.

  • GroupBy implements __getitem__, so as far as Python is concerned it is a mapping; however our implementation of this method is not in the sense of a mapping (it is for column selection)
  • GroupBy implements keys as a property; Python's dict construction is expecting keys to be a method. So when Python's dict constructor does grouped.keys(), it fails.
  • dict(iter(grouped)) works.

@Moisan Moisan force-pushed the pylint-unnecessary-comprehension branch from 3a91169 to 5831cd5 Compare November 13, 2022 00:21
@topper-123 topper-123 added the Code Style Code style, linting, code_checks label Nov 13, 2022
@topper-123 topper-123 added this to the 2.0 milestone Nov 13, 2022
@topper-123 topper-123 merged commit 6fac28d into pandas-dev:main Nov 13, 2022
mliu08 pushed a commit to mliu08/pandas that referenced this pull request Nov 27, 2022
* STYLE: fix pylint unnecessary-comprehension warnings

* fixup! STYLE: fix pylint unnecessary-comprehension warnings

* fixup! fixup! STYLE: fix pylint unnecessary-comprehension warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants