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

sort autofix incorrectly removes nested sorts #5712

Closed
BorisOrca opened this issue Jul 12, 2023 · 1 comment · Fixed by #5761
Closed

sort autofix incorrectly removes nested sorts #5712

BorisOrca opened this issue Jul 12, 2023 · 1 comment · Fixed by #5761
Labels
accepted Ready for implementation bug Something isn't working good first issue Good for newcomers help wanted Contributions especially welcome

Comments

@BorisOrca
Copy link

BorisOrca commented Jul 12, 2023

Minimal reproduction:

values = [(2, "b"), (2, "a"), (1, "b"), (1, "a")]
print(sorted(sorted(values, key=lambda x: x[1]), key=lambda x: x[0]))

prints out: [(1, 'a'), (1, 'b'), (2, 'a'), (2, 'b')]

After running ruff check reproduce_sort_issue.py
Found 1 error (1 fixed, 0 remaining).

The "fixed" code is:

values = [(2, "b"), (2, "a"), (1, "b"), (1, "a")]
print(sorted(values, key=lambda x: x[0]))

prints out: [(1, 'b'), (1, 'a'), (2, 'b'), (2, 'a')]

> ruff --version
ruff 0.0.277
@charliermarsh charliermarsh added bug Something isn't working accepted Ready for implementation labels Jul 12, 2023
@charliermarsh
Copy link
Member

Looks like we need to avoid collapsing sorts with different keys.

@charliermarsh charliermarsh added good first issue Good for newcomers help wanted Contributions especially welcome labels Jul 12, 2023
charliermarsh pushed a commit that referenced this issue Jul 14, 2023
## Summary

Nested calls to `sorted` can only be collapsed if the calls are
identical (i.e., they have the exact same keyword arguments).
Update C414 to only flag such cases.

Fixes #5712

## Test Plan

Updated snapshots.
Tested against flake8-comprehensions. It incorrectly flags these cases.
evanrittenhouse pushed a commit to evanrittenhouse/ruff that referenced this issue Jul 19, 2023
## Summary

Nested calls to `sorted` can only be collapsed if the calls are
identical (i.e., they have the exact same keyword arguments).
Update C414 to only flag such cases.

Fixes astral-sh#5712

## Test Plan

Updated snapshots.
Tested against flake8-comprehensions. It incorrectly flags these cases.
konstin pushed a commit that referenced this issue Jul 19, 2023
## Summary

Nested calls to `sorted` can only be collapsed if the calls are
identical (i.e., they have the exact same keyword arguments).
Update C414 to only flag such cases.

Fixes #5712

## Test Plan

Updated snapshots.
Tested against flake8-comprehensions. It incorrectly flags these cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation bug Something isn't working good first issue Good for newcomers help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants