-
Notifications
You must be signed in to change notification settings - Fork 113
Add the pyright language server #2797
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
yunyad
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 generally. I've left a few comments. However, please remove .gitignore and yarn.lock as they should not be submitted. Make only the necessary changes.
This reverts commit 515126b.
4325d51 to
48c4d43
Compare
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
yunyad
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
Yicong-Huang
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.
Thanks for the PR. The code needs some more change and please refer to the detailed comments.
For the PR description, please document the reason why we switch to pyright as the new default language server for python. The current reasoning "Its performance on detecting syntax and semantic error is better than pylsp" is too vague and maybe subjective. Please document the evaluations we did and references we found online to justify this. Make sure to use a neutral tone as this is an open source project, and comments are openly visible.
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/texera/web/TexeraWebApplication.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/amber/engine/common/AmberConfig.scala
Outdated
Show resolved
Hide resolved
Yicong-Huang
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. added some minor comments in code.
...scala/edu/uci/ics/texera/web/resource/pythonlanguageserver/PythonLanguageServerManager.scala
Show resolved
Hide resolved
...scala/edu/uci/ics/texera/web/resource/pythonlanguageserver/PythonLanguageServerManager.scala
Outdated
Show resolved
Hide resolved
...scala/edu/uci/ics/texera/web/resource/pythonlanguageserver/PythonLanguageServerManager.scala
Outdated
Show resolved
Hide resolved
|
We don't want to rely an external google doc for the comparison, as that URL may become obsolete. We could either include the comparison in this PR, or include it the corresponding blog. The former may be better. |
chenlica
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.
Please address my comments.
...scala/edu/uci/ics/texera/web/resource/pythonlanguageserver/PythonLanguageServerManager.scala
Outdated
Show resolved
Hide resolved
We could generate a PDF for the comparison, and include the PDF in this PR, if that's allowed. |
chenlica
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.
… UDF action (#2811) This PR introduces a new AI-based feature called "Add Type Annotation" that enhances the user experience of the Python UDF on top of the Pyright language server. The following are related PR's: -PR for the "Add the pyright language server" : #2797. -PR for the "[1/3] Add AI Assistant Service - Ai Flag" : #2808 -PR for the "[3/3] Add AI Assistant Service - add the "Add All Type Annotation" Python UDF action" : #2812 **Key changes:** Added a Monaco action in the frontend Python UDF editor to allow users to automatically add type annotations for an argument each time they select a single argument in their code. **Add Type Annotation:** This action requires the user to select a single argument in their Python UDF code. A UI with a suggestion for the selected argument will pop up. The user can choose to accept or decline the suggestion from OpenAI. If they accept the suggestion, it will be added to their code; otherwise, the code will remain unchanged. The suggestion is provided by OpenAI. After the backend receives the response from OpenAI, it will be forwarded to the frontend via a RESTful API. Example: https://github.com/user-attachments/assets/29cee6c8-c793-4d83-8b27-db64bcc636ae --------- Co-authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Co-authored-by: Yicong Huang <yicong.huang@observeinc.com>
…thon UDF action (#2857) This PR introduces another AI feature call "Add All Type Annotation" that enhance the user experience of the Python UDF after integrating with the Pyright language server. You can refer to the related PR here: -PR for the "Add the pyright language server" : #2797. -PR for the "[1/3] Add AI Assistant Service - Ai Flag" : #2808 -PR for the "[2/3] Add AI Assistant Service - add the "Add Type Annotation" Python UDF action" : #2811 **Key change:** Added another Monaco action in the frontend Python UDF editor to allow users to automatically add all type annotations for the arguments within their selected code block. **Add All Type Annotation:** This action requires the user to select a code block in their Python UDF code. The action will automatically identify all arguments without type annotations and provide type annotation suggestions for each argument sequentially. The selected code block will be sent to the backend for Python Abstract Syntax Tree (AST) analysis to locate arguments without type annotations, and the result will be sent back to the frontend via a RESTful API. If the user selects a code block that has no arguments or where all the arguments already have type annotations, the code will remain unchanged. Example: https://github.com/user-attachments/assets/d70c1b02-b309-4359-a267-0a5eef831316
This PR adds a flag to specify a language server for Python UDF. By default we use the `pyright` language server, and can switch to to the original language server called `pylsp` (python language server). **Key change**: -Add a flag to switch between `pyright` and `pylsp` -Add the `pyright` language server **When the flag is set to use the `pyright`**:  **When the flag is set to use the `pylsp`**:  **Reason to add `pyright`**: -Performance on the basic language server function: Both `Pyright` and `pylsp` show comparable performance in fundamental language server functionalities, such as hover, go-to definition, and autocompletion. Thus the basic user experience remains the same regardless of the chosen language server. -Pyright has a better ability to detect semantic errors: `Pyright` excels in identifying a broader range of syntax and semantic errors, particularly for code that includes type annotations. This strength makes it a valuable tool for developers who require more rigorous type checking and error detection in their Python code. **Evaluation between two language servers**: A detailed evaluation comparing the performance of two language servers on detecting basic semantic errors:  **Differences between two language servers**: For a more in-depth comparison of the differences between Pyright and pylsp, please refer to this discussion: python-lsp/python-lsp-server#90 **Summary**: While `Pyright` offers the potential for an improved user experience, especially in terms of finding semantic errors, `pylsp` is widely used and well-documented within its developer community. To balance the benefits of both tools, we have chosen to retain `pylsp` for ongoing maintenance while also introducing `Pyright`. This PR includes a flag that allows users to switch between the two language servers, providing flexibility based on individual project needs.
… UDF action (#2811) This PR introduces a new AI-based feature called "Add Type Annotation" that enhances the user experience of the Python UDF on top of the Pyright language server. The following are related PR's: -PR for the "Add the pyright language server" : #2797. -PR for the "[1/3] Add AI Assistant Service - Ai Flag" : #2808 -PR for the "[3/3] Add AI Assistant Service - add the "Add All Type Annotation" Python UDF action" : #2812 **Key changes:** Added a Monaco action in the frontend Python UDF editor to allow users to automatically add type annotations for an argument each time they select a single argument in their code. **Add Type Annotation:** This action requires the user to select a single argument in their Python UDF code. A UI with a suggestion for the selected argument will pop up. The user can choose to accept or decline the suggestion from OpenAI. If they accept the suggestion, it will be added to their code; otherwise, the code will remain unchanged. The suggestion is provided by OpenAI. After the backend receives the response from OpenAI, it will be forwarded to the frontend via a RESTful API. Example: https://github.com/user-attachments/assets/29cee6c8-c793-4d83-8b27-db64bcc636ae --------- Co-authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Co-authored-by: Yicong Huang <yicong.huang@observeinc.com>
…thon UDF action (#2857) This PR introduces another AI feature call "Add All Type Annotation" that enhance the user experience of the Python UDF after integrating with the Pyright language server. You can refer to the related PR here: -PR for the "Add the pyright language server" : #2797. -PR for the "[1/3] Add AI Assistant Service - Ai Flag" : #2808 -PR for the "[2/3] Add AI Assistant Service - add the "Add Type Annotation" Python UDF action" : #2811 **Key change:** Added another Monaco action in the frontend Python UDF editor to allow users to automatically add all type annotations for the arguments within their selected code block. **Add All Type Annotation:** This action requires the user to select a code block in their Python UDF code. The action will automatically identify all arguments without type annotations and provide type annotation suggestions for each argument sequentially. The selected code block will be sent to the backend for Python Abstract Syntax Tree (AST) analysis to locate arguments without type annotations, and the result will be sent back to the frontend via a RESTful API. If the user selects a code block that has no arguments or where all the arguments already have type annotations, the code will remain unchanged. Example: https://github.com/user-attachments/assets/d70c1b02-b309-4359-a267-0a5eef831316
This PR adds a flag to specify a language server for Python UDF. By default we use the
pyrightlanguage server, and can switch to to the original language server calledpylsp(python language server).Key change:
-Add a flag to switch between
pyrightandpylsp-Add the
pyrightlanguage serverWhen the flag is set to use the

pyright:When the flag is set to use the

pylsp:Reason to add
pyright:-Performance on the basic language server function: Both
Pyrightandpylspshow comparable performance in fundamental language server functionalities, such as hover, go-to definition, and autocompletion. Thus the basic user experience remains the same regardless of the chosen language server.-Pyright has a better ability to detect semantic errors:
Pyrightexcels in identifying a broader range of syntax and semantic errors, particularly for code that includes type annotations. This strength makes it a valuable tool for developers who require more rigorous type checking and error detection in their Python code.Evaluation between two language servers:

A detailed evaluation comparing the performance of two language servers on detecting basic semantic errors:
Differences between two language servers:
For a more in-depth comparison of the differences between Pyright and pylsp, please refer to this discussion:
python-lsp/python-lsp-server#90
Summary:
While
Pyrightoffers the potential for an improved user experience, especially in terms of finding semantic errors,pylspis widely used and well-documented within its developer community. To balance the benefits of both tools, we have chosen to retainpylspfor ongoing maintenance while also introducingPyright. This PR includes a flag that allows users to switch between the two language servers, providing flexibility based on individual project needs.