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

PR: Update README with information on Pyright/Pylance integration and add CLI subcommand to help generate the related config #450

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

krokosik
Copy link
Contributor

@krokosik krokosik commented Aug 24, 2023

My proposition of updated README and CLI that explains how to update the pyright configuration to eliminate type checking errors, especially in VSCode.

Solution based on this comment and follows the discussion from my issue.

NOTE:
I couldn't get the pre-commit hooks to run. It seems that some of them are outdated and raise this error that has been fixed in newer versions. After updating, however, the yamllint check was throwing errors instead, so I left it as is.

Fixes #447.

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work here @wkrasnicki ! Thank you so much for taking the time to not only update the README but also extend the CLI support to get the configs! I think the only missing element here is to add a test at https://github.com/spyder-ide/qtpy/blob/master/qtpy/tests/test_cli.py for the new CLI command but other than that this LGTM 👍

@dalthviz
Copy link
Member

NOTE:
I couldn't get the pre-commit hooks to run. It seems that some of them are outdated and raise this PyCQA/isort#2077 that has been fixed in newer versions. After updating, however, the yamllint check was throwing errors instead, so I left it as is.

That makes sense, we planned to use pre-commit here at some point (adding also a CI check for that) but that is still a pending task. Here the related issue: #345

I think we added the .pre-commit-config.yaml file while merging some other things 😅 but maybe could be worthy to remove it for the moment since it is throwing errors when used 🤔

What do you think @ccordoba12 @CAM-Gerlach ?

@dalthviz dalthviz changed the title Update Docs with information on Pyright/Pylance integration PR: Update Docs with information on Pyright/Pylance integration and add CLI subcommand to help generate the related config Aug 24, 2023
@krokosik
Copy link
Contributor Author

Ah, the power of tests. It actually caught a bug with boolean values not being lowercase in TOML 😅

@krokosik
Copy link
Contributor Author

Btw I have black formatter configured in my IDE to format on save and it looks like it changed the formatting a bit. Sorry for that, would you like me to revert these changes?

@dalthviz
Copy link
Member

Btw I have black formatter configured in my IDE to format on save and it looks like it changed the formatting a bit. Sorry for that, would you like me to revert these changes?

I did a quick check and seem small enough, so no problem with just leaving them 👍

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the work here @wkrasnicki !

@dalthviz dalthviz merged commit 8017a8f into spyder-ide:master Aug 24, 2023
12 checks passed
@ccordoba12
Copy link
Member

What do you think @ccordoba12 @CAM-Gerlach ?

I think that work should be finished since more and more people are contributing to this project. Formatting the code with Black and then adding pre-commit should be enough.

@krokosik
Copy link
Contributor Author

And thank you for all the support and prompt responses @dalthviz

@dalthviz dalthviz changed the title PR: Update Docs with information on Pyright/Pylance integration and add CLI subcommand to help generate the related config PR: Update README with information on Pyright/Pylance integration and add CLI subcommand to help generate the related config Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type hints not working for qtpy specific imports
3 participants