Pylint: Enhancement: Add a [unnecessary-comprehension]-checker

[pheanex]

I suggest adding a new [unnecessary-comprehension]-checker (name up for discussion).

Inspired by @treyhunner's blogpost Abusing and overusing list comprehensions in Python (Section "Using comprehensions when a more specific tool exists")
we could implement a checker that detects list/dict/set-comprehensions that can be replaced by the corresponding list/dict/set-constructors, which is faster and more readable.
For example:

• list(iterable) instead of [x for x in iterable]
• dict(some_dict) instead of {k: v for k, v in some_dict}
• set(iterable) instead of {e for e in iterable}

Although these cases seem trivial/obvious, I think having such a checker would help in cases where longer/more complex variable-names are used.

I'd be willing to implement the checker myself.

What do you think?

As a next step and if there are no basic objections I would add testcases here.

[PCManticore]

Sound good and useful, thanks for tackling this.

[pheanex]

I came up with the following testcases:

# For name-reference see https://docs.python.org/3/reference/expressions.html#displays-for-lists-sets-and-dictionaries

# List comprehensions
[x for x in iterable]  # [unnecessary-comprehension]
[y for x in iterable]  # expression != target_list
[x for x,y,z in iterable]  # expression != target_list
[(x,y,z) for x,y,z in iterable]  # [unnecessary-comprehension]
[(x,y,z) for (x,y,z) in iterable]  # [unnecessary-comprehension]
[x for x, *y in iterable]  # expression != target_list
[x for x in iterable if condition]  # exclude comp_if
[y for x in iterable for y in x]  # exclude nested comprehensions

# Set comprehensions
{x for x in iterable}  # [unnecessary-comprehension]
{y for x in iterable}  # expression != target_list
{x for x,y,z in iterable}  # expression != target_list
{(x,y,z) for x,y,z in iterable}  # [unnecessary-comprehension]
{(x,y,z) for (x, y, z) in iterable}  # [unnecessary-comprehension]
{x for x, *y in iterable}  # expression != target_list
{x for x in iterable if condition}  # exclude comp_if
{y for x in iterable for y in x}  # exclude nested comprehensions

# Dictionary comprehensions
{k: v for k, v in iterable}  # [unnecessary-comprehension]
{v: k for k, v in iterable}  # key value wrong order
{k: v for (k, v) in iterable}  # [unnecessary-comprehension]
{x: y for x,y,z in iterable}  # expression != target_list
{x[0]: x[1] for *x in iterable}  # [unnecessary-comprehension]
{x: y for x, y in iterable if condition}  # exclude comp_if
{y: z for x in iterable for y, z in x}  # exclude nested comprehensions

Any feedback on the testcases?
=> I'd start implementing the checker, otherwise.

[treyhunner]

@pheanex just checking old notifications and saw this. I like this idea. 💖

I don't think this one should be seen as an unnecessary comprehension:

{x[0]: x[1] for *x in iterable}

I doubt that'd come up often, but it's possible that's an iterable of 3-item tuples, so dict(iterable) wouldn't be equivalent in that case.

[feluelle]

Can anyone explain to me why {k: v for k, v in enumerate(_list)} is a unnecessary-comprehension. What would be better?

[feluelle]

Okay - just dict(enumerate(_list)) works for pylint but now PyCharm is complaining about Unexpected type(s):

[feluelle]

hmm.. iter of str works but not list of str for the dict constructor. That's a PyCharm issue then, because every list is an iterable?

[PCManticore]

@feluelle I'm not sure I understand your last comment, do you mean PyCharm complains on something like dict(list(enumerate("test"))) or that pylint complains?

[jdhao]

@PCManticore @pheanex I am seeing the unnecessary-comprehension warning for the following code:

labels = ['one', 'two', 'three']                             
indexed_labels = [(i, text) for i, text in enumerate(labels)] # warning here

I am currently using pylint 2.4.4. I think it is a false positive. Otherwise, what are the better alternatives?

[feluelle]

@jdhao try list(enumerate(labels))

https://www.geeksforgeeks.org/python-convert-list-to-indexed-tuple-list/

[jdhao]

@feluelle Thanks for the tip. I find the longer version more readable and clear, though.

[devenpatel2]

@PCManticore @pheanex
I don't think this should be getting a warning. A comprehension seems like a reasonable thing to do something like this.

list_tups = [('a', 2), ('b', 3)]
converted =  {k:v for k, v in list_tups}

[pheanex]

@devenpatel2 don't you think using dict instead is simpler/easier to read?
So instead of

converted =  {k:v for k, v in list_tups}

it would be

converted = dict(list_tups)

[devenpatel2]

Goodness me! I never tried that. This is so much elegant. Thank you.