-
Notifications
You must be signed in to change notification settings - Fork 128
python: bootstrap pyrefly extension #10647
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
|
E2E Tests 🚀 |
| "python.analysis.typeCheckingMode": "off", | ||
| "ruff.importStrategy": "useBundled" | ||
| "ruff.importStrategy": "useBundled", | ||
| "python.pyrefly.displayTypeErrors": "force-on", |
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.
The default behavior is to not show type-checking errors unless you have a pyrefly.toml or pyproject.toml with a [tool.pyrefly] section. This setting will make it always show type-checking errors. What do you think about this? I think they were off for pyright.
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.
I'd prefer the "not show type-checking errors unless you have a pyrefly.toml or pyproject.toml with a [tool.pyrefly] section" option, just to stay on the less-aggressive side of diagnostics.
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.
Yeah I am rethinking it and like this idea better.
| "ruff.importStrategy": "useBundled", | ||
| "python.pyrefly.displayTypeErrors": "force-on", | ||
| "[python]": { | ||
| "editor.inlayHints.enabled": "offUnlessPressed" |
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.
This turns off inlay hints for Python files unless you hold down Ctrl+Option, which is kinda cool.
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.
The code itself looks good! A few things I kicked the tired on, in .py scripts and notebooks:
- general completion/syntax highlighting/clicking around
- magic completion
x??goto definition- hovering
- autocomplete
- finding references
- renaming symbols
A few notes. I don't think they're blocking, but noting what I saw!
Should the highlighting be in the console as well? I see the old behavior there and in Quarto Python chunks.
FYI, I needed to clear state and rerun sh rebuild.sh to pick up these changes. I haven't tested a local build, but I'm guessing that pyrefly will download, but pyright will stay installed until a user uninstalls it or resets Positron's state. I think this is the right choice, but we may want to document that folks will have to do some finagling if there are conflicts.
|
I think the failing test is unrelated.
I'll investigate! I don't want this PR to cause any regressions upon merge, but if there's something that's still missing that wasn't there before (e.g. better syntax highlighting in the console) I'll open tickets for future enhancements.
Good point, we'll check out what happens with tomorrow's daily build and document a recommendation in the release notes. |
|
Investigating the missing features - I opened #10664 to do later and made sure there existed an issue for missing semantic tokens in Quarto docs: quarto-dev/quarto#420 I will look at the failing test again to make sure it's not related. |
|
Yes @ntluong95 see #10305! (slated for later this week) |
|
Code changes look good! |

Addresses #3731 and #10304. Switches the bootstrapped Pyright extension to Pyrefly.
Release Notes
New Features
Bug Fixes
QA Notes
Try the LSP features, make sure the new default settings work, etc.