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

Added new checks C413, C414, C415, and C416. #190

Merged
merged 7 commits into from Nov 15, 2019
Merged

Added new checks C413, C414, C415, and C416. #190

merged 7 commits into from Nov 15, 2019

Conversation

ngnpope
Copy link
Contributor

@ngnpope ngnpope commented Oct 30, 2019

Hi!

I've added a few additional checks that I think are helpful:

@@ -18,7 +18,7 @@ def test_version(flake8dir):
def test_C400_pass_1(flake8dir):
flake8dir.make_example_py(
"""
foo = [x for x in range(10)]
foo = [x + 1 for x in range(10)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that many of the tests needed tweaking so as not to violate the new C416 rule.

Comment on lines +945 to +946
# Column offset for list comprehensions was incorrect in Python < 3.8.
# See https://bugs.python.org/issue31241 for details.
col_offset = 1 if sys.version_info >= (3, 8) else 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈

@adamchainz
Copy link
Owner

Thanks for these - I'll get around to reviewing soon!

Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Can you open up your branch for maintainer changes? I can't push to it and have done some small changes locally

msg = (
msg.rstrip(".")
+ " - toggle reverse argument to sorted()."
)
Copy link
Owner

Choose a reason for hiding this comment

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

I couldn't follow this so I've rewritten it with a for loop and an extra message piece.

@ngnpope
Copy link
Contributor Author

ngnpope commented Nov 13, 2019

Can you open up your branch for maintainer changes?

Weird. The box is ticked (which is the default). Will try toggling it off and on.

@adamchainz
Copy link
Owner

It was my fault - my git invocation was off so I was trying to push to a branch in your repo called pope1ni/improvements rather than just improvements 🤷‍♂️

Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Thanks very much!

The only thing I'd change is to move to one test per case like the other tests. Batching them does increase speed with flake8 in subprocesses but if something fails it's harder to track down the cause. Anyway I think I'll have a pass at simplifying all the tests. Will post again when this is released.

@adamchainz adamchainz merged commit f2aeae2 into adamchainz:master Nov 15, 2019
@adamchainz
Copy link
Owner

Released in 3.1.0: https://pypi.org/project/flake8-comprehensions/3.1.0/

@adamchainz
Copy link
Owner

🎺 https://twitter.com/AdamChainz/status/1195284920798777344

@ngnpope ngnpope deleted the improvements branch November 15, 2019 14:13
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.

Unnecessary list comprehension that can be a call to list() Add new rule for reversed(sorted(...))
2 participants