Skip to content
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

[Enhancement]: Add operator replacement for operator.itemgetter and operator.attrgetter #321

Closed
Skylion007 opened this issue Jan 1, 2024 · 3 comments · Fixed by #322
Closed
Labels
enhancement New feature or request

Comments

@Skylion007
Copy link

Overview

Expand operator replacement rules with operator.itemgetter / operator.attrgetter

Proposal

Mostly people do not know that operator.itemgetter and operator.attrgetter exist. They can be often very useful especially for sorting. For example:

a = sorted(b, key=lambda x: x[1])

becomes

a = sorted(b, key=operator.itemgetter(1)

This becomes even more important in complex situations where you accessing multiple attributes or items at once. See https://stackoverflow.com/questions/11287207/why-should-i-use-operator-itemgetterx-instead-of-x for examples where it can help.

@Skylion007 Skylion007 added the enhancement New feature or request label Jan 1, 2024
@dosisod
Copy link
Owner

dosisod commented Jan 2, 2024

I think this would be a good addition, especially if you are doing something like this:

a = sorted(b, key=lambda x: (x[3], x[2], x[1], x[0]))
# vs
a = sorted(b, key=itemgetter(3, 2, 1, 0))

One concern I have is that attrgetter("attr") won't give a Mypy error if attr happens to get renamed to something else, whereas lambda x: x.attr will give an error that attr doesn't exist anymore. I don't think this is a major problem though, since proper testing would be able to weed this out.

@Skylion007
Copy link
Author

Skylion007 commented Jan 2, 2024

Yeah, they could maybe different rules then or something. itemgetter seems to have no real downsides though. I could imagine someone may want to enable the itemgetter rule, but not the attrgetter.

dosisod added a commit that referenced this issue Jan 13, 2024
dosisod added a commit that referenced this issue Jan 13, 2024
dosisod added a commit that referenced this issue Jan 13, 2024
@dosisod
Copy link
Owner

dosisod commented Jan 13, 2024

Thank you @Skylion007 for opening this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants