Colon after code causes issue

[radoering]

Due to the fact that several tools are involved in this problem, I am not 100% sure that the problem is here, but at least I think so. 😉

I'll try to describe the complete use case:

We don't like that imports from typing are forced into a type checking block so they need to be quoted (see python-poetry/poetry#4776 (comment)). (Maybe, something to discuss in another issue. 😉)

Thus, we add # noqa: TC002 to such imports. So far, so good.

However, yesqa keeps removing these noqa comments. At first, I thought this to be an issue of yesqa.
So, I debugged it. And it turns out that yesqa is calling flake8 --format=%(code)s <myfile>.

For example, if you run this on a file with the following content

from typing import List


def f(a: List[int]):
    myDict = {1 + 1 : 2}
    print(myDict)
    print(a)

you get

TC002:
E203

If you print the full message, you can see that the builtin messages do not have a colon. Of course, yesqa could strip the colon but I think that %(code)s should only return the code (without a colon). I did not dig deeper from this point. Maybe, it would be best to remove the colon? I'll attach a corresponding PR in a moment.

[sondrelg]

We don't like that imports from typing are forced into a type checking block so they need to be quoted (see python-poetry/poetry#4776 (comment)). (Maybe, something to discuss in another issue. 😉)

Well the core idea behind this plugin is that imports for type annotations should not be scoped in a way where they're imported during the runtime of you app or library. Running this code in prod is a waste of cpu-cycles - though admittedly, the typing package imports are nowhere near as bad as some library imports. Instead they should be scope in a way where they're included for static type checkers, but not for general execution. Let me know if this point is unclear, because it's really fundamental 🙂

That being said, there might be room to add a plugin setting for opting out of checking typing imports - otherwise you might end up with a lot of noqas.

Personally I prefer using from __futures__ import annotations (TC1 error range) to not have to quote any imports. If you have the option, this might be something to consider. Let me know if you want me to expand on this in any way 👍

I think that %(code)s should only return the code (without a colon)

Yeah this seems reasonable. Happy to review and accept a PR for this, if this solves your issue 🙂

[radoering]

Personally, I prefer using from __futures__ import annotations, too. Unfortunately, that is not (yet) an option until Python 3.6 support is dropped in the concerning project.

In general, I can see your point on the scope of imports. I am not an expert in this topic, but I don't think this point can be applied to typing imports. I have to from typing import TYPE_CHECKING at global scope anyway. Due to the fact, that typing seems to be a module and not a package, there should be (almost?) no overhead when importing another symbol from that module, shouldn't it?

Even if there is no overhead due to the import, I have no idea if there is an overhead in having a type annotation like List compared to "List" (assuming from __futures__ import annotations is not used). Do you know?

[sondrelg]

there should be (almost?) no overhead when importing another symbol from that module, shouldn't it?

Yeah I think this is right. I'm also no expert, I primarily wrote this for a logical separation of imports after introducing mypy into a very large project at work. The addition of type hints would add 10-40 imports in large files, and it just got very hard to work with.

If you want, we could add a setting to allow specific module imports - like a whitelist/passlist for modules exempt from checking. This seems like a flexible way of solving the issue. I've not considered how hard it would be to implement, but it doesn't sound too hard.

Also, 1.0.5 should be out by now, if you want to test it solves the yesqa issue 👍

[radoering]

Thanks for the super fast reaction. The yesqa issue is solved with 1.0.5.👍

Imho such a setting would be nice. Just to be sure, I asked a poetry core member to comment here if this meets his expectations.

[finswimmer]

Hello everyone,

an option to allow typing imports outside the type-checking block would be great. I've no idea how hard it is to implement a customizable whitelist. I guess an option to allow typing (and maybe typing-extensions) would be great as well.

Thanks a lot for this very useful plug in!

fin swimmer

[sondrelg]

I will look into it after work today. I use Poetry in all my projects, so happy to contribute back in any way 🎉

I'll keep this issue open for now, and add relevant updates here. Hopefully it's an easy fix and I can release a new minor version tonight.

[sondrelg]

I've added a new setting in #51 now which I hope should do the trick. Before I release the new version, would you mind testing the branch to see if you immediately spot any issues @radoering?

You should be able to install it with pip install git+https://github.com/snok/flake8-type-checking@exempt-modules, and to ignore typing and typing-extensions, you must specify the new setting in your setup.cfg (or equivalent) like this:

[flake8]
type-checking-exempt-modules = typing, typing-extensions

Remember to run it with flake8, not using pre-commit, since the pip install won't actually affect the pre-commit environment (I just made that mistake when testing locally) 😌

---

Also, If any of you want to look at the PR, feel free. The setting naming, etc., is all a bit arbitrary right now, so feedback is always welcome 👏

[radoering]

I checked the branch and could not find any negative side effects: The # noqa for typing imports are not required anymore and other violations are still found. 👍

Just a weird observation (that has nothing to do with the change): At first, I installed flake8 and flake8-type-checking in the virtualenv of the project, running flake8 without pre-commit and noticed that some TC002 became TC001. Then, I injected the branch in the pre-commit environment and noticed that the TC002 remained TC002. On closer inspection, it seems that there are no TC001 at all (even for internal imports) when executing via pre-commit. I am not sure if that has something to do with pre-commit vs. not pre-commit or if the reason is that I did not install all the other flake8 plugins that are used in pre-commit of poetry.

[sondrelg]

Nice, I'll release v1.1 now then - it should be ready in ~1 minute 🙂

As for the weirdness, it sounds like you're on to something. This function is what separates TC001 from TC002 and is a little weird maybe. I don't think it matters unless you have a good reason to want only one of the errors, since they're pretty much interchangeable, but I'll definitely keep it in mind - perhaps there something that can be done. If you have any ideas, PRs are always welcome of course.

Hope things work out with the new release. I'll close this for now, but feel free to reopen or open a new issue if anything else comes up 👏

[finswimmer]

Awesome @sondrelg 👍 Thanks a lot 🙏