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

Add C417 - Unnecessary use of map #409

Merged
merged 9 commits into from
May 19, 2022

Conversation

tusharsadhwani
Copy link
Contributor

Resolves #405

@tusharsadhwani
Copy link
Contributor Author

tusharsadhwani commented Mar 12, 2022

Can I also add set comprehensions for set(map(...))? I'm not entirely sure if it makes sense

@adamchainz
Copy link
Owner

I think it makes sense - and dict(map()) as well, if you could! They could also all be one error code with a templated message, IMO.

I haven't found time to review this properly but it looks good.

@tusharsadhwani
Copy link
Contributor Author

@adamchainz Merged it all into C417, and added set and dict as well.

@tusharsadhwani tusharsadhwani changed the title Add C417 and C418 Add C417 - Unnecessary use of map Apr 14, 2022
@sbrugman
Copy link
Contributor

What is the status on this one?
(Would be neat to have C417 if C418/C419 are accepted)

README.rst Outdated
Comment on lines 166 to 169
``map(f, iterable)`` has great performance when ``f`` is a built-in function,
and it makes sense if your function already has a name. But if you need to
introduce a lambda, it's more readable and faster to use a generator expression
or a comprehension -- no function calls needed.
Copy link
Owner

Choose a reason for hiding this comment

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

I'm tweaking the wording, most notably to remove the readability claim - people love to debate what's readable, but we can be absolutely sure that map is slower 😅

Comment on lines 778 to 805
def test_C417_fail_1(flake8_path):
(flake8_path / "example.py").write_text(
dedent(
"""\
map(lambda x: x * 2, iterable)
list(map(lambda x: x * 2, iterable))
set(map(lambda num: num % 2 == 0, nums))
dict(map(lambda v: (v, v ** 2), values))
"""
)
)
result = flake8_path.run_flake8()
assert result.out_lines == [
"./example.py:1:1: C417 Unnecessary use of map - "
"use a generator expression instead.",
"./example.py:2:1: C417 Unnecessary use of map - "
"use a list comprehension instead.",
"./example.py:3:1: C417 Unnecessary use of map - "
"use a set comprehension instead.",
"./example.py:4:1: C417 Unnecessary use of map - "
"use a dict comprehension instead.",
]
Copy link
Owner

Choose a reason for hiding this comment

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

I will split this test - I deliberately want separate test cases so it's easy to see what fails/passes whilst editing code.

@adamchainz adamchainz merged commit 404158c into adamchainz:main May 19, 2022
@adamchainz
Copy link
Owner

Released in 3.10.0, thanks!

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.

Convert map(lambda x: expression, iterable) to (expression for x in iterable)
3 participants