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

Failed to create fix for UnnecessaryMap: Currently not supporting default values #6715

Closed
qarmin opened this issue Aug 21, 2023 · 8 comments · Fixed by #6752
Closed

Failed to create fix for UnnecessaryMap: Currently not supporting default values #6715

qarmin opened this issue Aug 21, 2023 · 8 comments · Fixed by #6752
Assignees
Labels
bug Something isn't working fixes Related to suggested fixes for violations fuzzer Surfaced via fuzzing.

Comments

@qarmin
Copy link

qarmin commented Aug 21, 2023

Ruff 0.0.285

ruff  *.py --select ALL

file content:

_ = f"{dict(map(lambda v: (v, v**2), nums))}"
# For simple expressions, this could be: `(x if x else 1 for x in nums)`.
map(lambda x=1: x, nums)

error:

error: Failed to create fix for UnnecessaryMap: Currently not supporting default values

For me, this error should not be visible for user, because this message is useless for it

PY_FILE_TEST_178904787.py.zip

@dhruvmanila
Copy link
Member

The fix might not work if there are any falsy values in the iterable. For example, if one of the values of nums is 0 then it would use 1 but it explicitly passed the argument.

I do agree that the message isn't useful from user perspective and instead we should avoid auto-generating the fix.

@dhruvmanila dhruvmanila added bug Something isn't working fixes Related to suggested fixes for violations labels Aug 21, 2023
@charliermarsh charliermarsh added the fuzzer Surfaced via fuzzing. label Aug 21, 2023
@charliermarsh
Copy link
Member

Yeah it's fine that this is unsupported, but we should attempting to fix it and thus raising the message.

@charliermarsh
Copy link
Member

The fix might not work if there are any falsy values in the iterable. For example, if one of the values of nums is 0 then it would use 1 but it explicitly passed the argument.

Can you expand on this with an example?

@dhruvmanila
Copy link
Member

Here, you can see the output is different. Default values in this context will never be used as map will always pass a value to the function.

In [1]: nums = [0, 1, 2]

In [2]: list(map(lambda x=1: x, nums))
Out[2]: [0, 1, 2]

In [3]: list(x if x else 1 for x in nums)
Out[3]: [1, 1, 2]

@charliermarsh
Copy link
Member

But, isn't this suggesting that the user change list(map(lambda x=1: x, nums)) to [x for x in nums]? Why the inclusion of if x else 1?

@dhruvmanila
Copy link
Member

Oh, it's mentioned in the PR description:

For simple expressions, this could be: (x if x else 1 for x in nums).

But yeah, your suggestion would work correctly.

@charliermarsh
Copy link
Member

We should probably just skip attempting the fix if there are default values. It's an unusual pattern and I can think of a few cases where it would cause problems.

@dhruvmanila
Copy link
Member

Yeah, I'm thinking of the same solution.

@dhruvmanila dhruvmanila self-assigned this Aug 22, 2023
dhruvmanila added a commit that referenced this issue Aug 22, 2023
## Summary

Avoid `C417` for `lambda` with default and variadic parameters.

## Test Plan

`cargo test` and checking if it generates any autofix errors as test
cases
for `lambda` with default parameters already exists.

fixes: #6715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixes Related to suggested fixes for violations fuzzer Surfaced via fuzzing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants