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

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

Conversation

allisonkarlitskaya
Copy link

This essentially reverts 83df547.

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 tomv564#12. cc @eliwe.

This essentially reverts 83df547.

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 python-lsp#81
@Richardk2n
Copy link
Member

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

@allisonkarlitskaya
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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

@Richardk2n Richardk2n closed this Sep 3, 2024
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.

default config causes mypy cache invalidation due to --follow-imports silent
2 participants