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

Fix #96 none handling for select and extend-select #98

Merged
merged 1 commit into from Jul 13, 2022
Merged

Fix #96 none handling for select and extend-select #98

merged 1 commit into from Jul 13, 2022

Conversation

wyuenho
Copy link
Contributor

@wyuenho wyuenho commented Jul 10, 2022

I'm not exactly sure if this is the best way to fix this issue, but this seems to be the only way to fix without haven't to rewrite a whole lot of tests.

@codecov
Copy link

codecov bot commented Jul 10, 2022

Codecov Report

Merging #98 (3e454d2) into main (7a5f727) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main     #98   +/-   ##
=====================================
  Coverage   96.9%   96.9%           
=====================================
  Files          3       3           
  Lines        420     421    +1     
=====================================
+ Hits         407     408    +1     
  Misses        13      13           
Impacted Files Coverage Δ
flake8_type_checking/plugin.py 85.0% <100.0%> (+0.3%) ⬆️

@sondrelg
Copy link
Member

Thanks for opening the issue and PR @wyuenho! Just so I can understand the issue properly, do you have a way for me to replicate the issue locally when on the flake8 main branch?

@wyuenho
Copy link
Contributor Author

wyuenho commented Jul 11, 2022

do you have a way for me to replicate the issue locally

Just install flake8 HEAD and flake8-type-checking in any project, write a .flake8 config without a select value, which is allowed, and that's it.

@sondrelg
Copy link
Member

Running

deactivate \
  && rm -rf venv \
  && python3 -m venv venv --upgrade-deps \
  && source venv/bin/activate \
  && pip install git+https://github.com/pycqa/flake8@main flake8-type-checking \
  && echo "[flake8]\nexclude = .venv" > .flake8 \
  && echo "print('test')" > main.py \
  && flake8 .

I'm not able to replicate on Python 3.9. What's missing?

@wyuenho
Copy link
Contributor Author

wyuenho commented Jul 11, 2022

Ah ok, so for some reason this code path only gets hit when the file contains something like this:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from collections.abc import AsyncGenerator

I'm not entirely sure why...

@sondrelg
Copy link
Member

Even with the example above I was not able to reproduce the issue yesterday. Would you be able to provide step-by-step instructions for how to replicate?

I'll get home from holidays tomorrow, and will have a bit more time available then. If you're able to provide replicating steps, I'll review and merge by Thursday most likely 👍 Thanks again for opening the PR!

@sondrelg
Copy link
Member

After reviewing the PR, this seems sensible regardless of whether I'm able to replicate or not 🙂 Thanks!

@sondrelg sondrelg merged commit 9bcbefe into snok:main Jul 13, 2022
@wyuenho
Copy link
Contributor Author

wyuenho commented Jul 13, 2022

I can reproduce this pretty consistently on both 3.9 and 3.10. I would check your pip caches and hardcode a commit hash into the package version instead of a tag and try again.

Thanks for merging! I guess now what's left is actually updating the readme to reflect the recommended way to config the rules for flake8 >= 4, which is extend-select.

@wyuenho wyuenho deleted the fix-96 branch July 13, 2022 20:22
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.

None yet

2 participants