search

python-lsp / pylsp-mypy

Mypy plugin for the Python LSP Server.
MIT License
118 stars 35 forks source link

Remove '--follow-imports silent' from mypy args #82

Closed allisonkarlitskaya closed 1 week ago

allisonkarlitskaya commented 4 months ago

This essentially reverts 83df547c5036116a411688edf016c0d32d788bd4.

The main reason for doing this is that this is not the default option when running mypy, which means that (absent other configuration) running mypy . and using pylsp-mypy will invalidate each others caches, which will cause both things to slow down.

--follow-imports silent is also incompatible with dmypy, so working around this issue by specifying follow_imports = 'silent' in your pyproject.toml is a non-starter if you want to use dmypy.

Closes #81

This would effectively be a revert to https://github.com/tomv564/pyls-mypy/pull/12. cc @eliwe.

Richardk2n commented 4 months ago

Is there a reason for using mypy on the command line, if you already have it running via pylsp-mypy in your IDE?

allisonkarlitskaya commented 4 months ago

Our tests include a mypy run, and I run it before I push. Sometimes a change made in one file causes errors in the files that use it, and it's good to find out about that before pushing.

allisonkarlitskaya commented 2 months ago

Gentle ping?

I'd like to see #81 fixed.

This is one possible solution (and I think it's the one that makes most sense), but maybe there are other proposals to pursue instead?

Richardk2n commented 2 months ago

While your PR likely breaks nothing as there is a guard already in place to filter out output from other files, avoiding this output in the first place (and potential associated runtime cost) seems to be sensible for me. In that regard the reasoning for the introduction of this flag still stands. You can turn it off using overrides. So either way it works, this is just about style?

allisonkarlitskaya commented 2 months ago

It's more about the default config being broken (if you care about caching): mypy run from the lsp with the default config will invalidate the cache of mypy from the CLI with the default config.

Richardk2n commented 1 month ago

How would you feel about making follow-imports a direct setting like strict, this way you do not have to deal with the clunky overrides system and I can keep the defaults that are most sensible to the operation of pylsp-mypy?

allisonkarlitskaya commented 1 month ago

How would you feel about making follow-imports a direct setting like strict, this way you do not have to deal with the clunky overrides system and I can keep the defaults that are most sensible to the operation of pylsp-mypy?

I think it would be an acceptable compromise but a little bit strange from pylsp-mypy's standpoint: after all, this setting doesn't really have an observable effect on pylsp itself (aside from the performance thing).

I'd be happy to use it in any case, though.

Richardk2n commented 1 month ago

Yes, you are correct, it is a little strange. However, I do not want to change the defaults, as they are sensible for pylsp-mypy, but I recognize, that an option to check all files without invalidating cache is sensible. I have no other idea to fulfill both.

Richardk2n commented 1 week ago

I have implemented the new option and released it. I hope this suits your need.