Skip to content

Expose generic mypy error as a diagnostic #40

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

Merged
merged 5 commits into from
Oct 9, 2022

Conversation

rchl
Copy link

@rchl rchl commented Sep 16, 2022

Generic mypy error was only visible to the user when running pylsp with --verbose --verbose.

It took me way too much time to figure out why pylsp_mypy is not producing the same errors as the command-line variant did. It turned out the error was:

/Applications/Sublime Text.app/Contents/MacOS/Lib/python33 is in the MYPYPATH. Please remove it.
See https://mypy.readthedocs.io/en/stable/running_mypy.html#how-mypy-handles-imports for more info

but that was only visible after I went and looked at pylsp_mypy source code and added some logs (didn't know about --verbose --verbose exposing that information at the time).

That motivated me to expose this error as a diagnostic on the first line.

Also aligned local mypy configuration with one used on CI to get consistent results.

@rchl rchl force-pushed the fix/generic-error branch from 611e67f to 03975b7 Compare September 16, 2022 18:13
@Richardk2n
Copy link
Member

As far as I remember these outputs are often warnings rather than errors. Therefore I am not sure if I want to always expose it. Is your experience different in that regard?

Also in regard to the CI configuration:
The line length for this project is actually 100 which is enforced by black. Thank you for pointing that out, that really should be made more consistant.
Can you give me a quick explanation why you put it into the setup config? When does it make a difference? And why not the pyproject.toml (I honestly don't know)

@rchl
Copy link
Author

rchl commented Sep 22, 2022

As far as I remember these outputs are often warnings rather than errors. Therefore I am not sure if I want to always expose it. Is your experience different in that regard?

The mypy output in this case is literally what I've quoted and it comes from stderr and breaks linting so I don't think it should be considered a warning.

But I guess what I should do to be safe is to create an error diagnostic if return code was non-0 and otherwise a warning.

Can you give me a quick explanation why you put it into the setup config? When does it make a difference? And why not the pyproject.toml (I honestly don't know)

Not sure apart from the fact that adding as [tool.flake8] in pyproject.toml doesn't seem to affect local flake8. But I can read up on that.

@rchl
Copy link
Author

rchl commented Sep 22, 2022

Flake8 cannot be configured via pyproject.toml, even though virtually all other Python dev tools have adopted it as the central location for project configuration. The

From: https://pypi.org/project/Flake8-pyproject/

@rchl
Copy link
Author

rchl commented Oct 5, 2022

ping

@Richardk2n
Copy link
Member

I have been away for the past 2 weeks. Your contribution is appreciated. I will have a detailed look and test it on the weekend.

@Richardk2n
Copy link
Member

The code has become quite complex. For now, I am ok with increasing the limit (that we did not adhere to anyway). For the future, I would like to try to get this down.

@Richardk2n Richardk2n merged commit f12354d into python-lsp:master Oct 9, 2022
@rchl rchl deleted the fix/generic-error branch October 9, 2022 19:19
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.

2 participants