-
Notifications
You must be signed in to change notification settings - Fork 482
feat(python-lsp)!: replace the old pylsp with zuban and ruff #1528
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
Conversation
See [previous PR](#1489) for details
|
does |
As mentioned in #1528, keep the ruff config align with previous config. This includes: 1. set default line length to 88, as `black`'s default behavior 2. align with previous linter config As for the pyrefly lsp config, I guess the default is good to go.
Thanks for your kindly remind, I check up the previous config for pylsp, and pyrefly config docs, and ruff config doc. |
|
Another thing is that, I suggest we should ask other maintainers for their opinion on this PR about whether its time to switch and what can we improve about lsp/linter/formatter config. |
|
Maybe I can't review it now.
|
|
Now there's another option for Python LSP, Zuban claims to support more lsp features, and look promising. We should take one that into consideration as well. |
|
I saw it as well, but it's kinda late (UTC+8 1a.m.) now, tomorrow I will try to test it. |
|
I guess for my use case for scripting in python with types, the mentioned refactor feature is working properly and all other things are working as intended. So I guess its ready for review and test. Another thing is the new zuban lsp, should we consider it instead of pyrefly? ref: @misumisumi @charliie-dev |
|
Found this amazing test set from python/typing, I will try to re-test, and then I guess we can choose a base on this test. |
|
results.html |
can we add ty to the list? |
Guess nothing is working on that one. Even basic array type infer. |
|
then +1 for zuban, lets gooo |
|
I also think it would be better to choose zuban now. P.S. |
As for the personal project problem, the current solution python-lsp is based on jedi as well, so I guess the community solution is absolutely fine. Personally, I would like the solution of using zuban for now. In the long term, if there's better solution with python lsp, I guess it wouldn't be a problem to switch to it. P.S. I happen to know one of the contributors of zuban and pyrefly, @asukaminato0721. The situation about all mentioned lsp inplementations is as this blog (Simplified Chinese) suggests. |
As [PR 1528] discussion suggests, zuban is more implemented and has less memory footprint.
|
So maybe we could merge this? @charliie-dev @misumisumi |
…_list` Signed-off-by: Charles Chiu <[email protected]>
charliie-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just tested on my private python repo, seems right lol
LGTM
|
imo, both are excellent choices. Zuban did very well in the standard. Pyrefly just released a beta version and will be the CI tool for PyTorch. d = {"2": 2}
if d.get("1") is None:
raise
x = d.get("1")Now only Pyrefly can infer that x is int. facebook/pyrefly@70b058e |
misumisumi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 💯


Note
See previous PR #1489 and #1172 for previous details.
Due to pyrefly is still not reached beta yet, we choose to wait its first beta release, as it doesn't have any comlicated refactor capibility for now.
The PR now is converted into a draft now, the information about pyrefly could retrived here and facebook/pyrefly#344. And this would likely to be a BREAKING CHANGE once merged.
Summary
As we are searching for a better Python LSP, and there has been updates on pyrefly as a better choice for this, which is now mature and solid, I think its time to move on for pyrefly as the default python lsp.
Details