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: Pass paths from the Pythonpath manager to the Pylint plugin #21891

Merged
merged 4 commits into from
Nov 28, 2024

Conversation

znapy
Copy link
Contributor

@znapy znapy commented Mar 14, 2024

Description of Changes

Add the PYTHONPATH environment from pythonpath-manager to the pylint plugin to access the root of the project without packages.

Example:

Suppose, you have a project with a single script (for simplicity in a spyder environment):

my-project
| - do_something.py

and you write a test:

my-project
| - do_something.py
| - tests
  | - test_do_something.py

In test_do_something.py you access to import do_something. So if you start Source -> Run code analysis it will show the error Unable to import 'do_something'. But in the IPython console !pylint tests/test_do_something.py doesn't show this error because there is PYTHONPATH environment.

And you should take additional steps to solve this problem, for example, make a packages:

my-project
| - my_project
  | - __init__.py
  | - do_something.py
  | - tests
    | - __init__.py
    | - test_do_something.py

or fix paths in pylintrc or pyproject.toml (section [tool.pylint], value init-hook=...):

my-project
| - do_something.py
| - tests
  | - test_do_something.py
| - pyproject.toml

No hacks are needed with this PR - it works out of the box.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @Znapy

@pep8speaks
Copy link

pep8speaks commented Mar 14, 2024

Hello @znapy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-25 12:50:28 UTC

@znapy znapy force-pushed the pypath4pylint branch 2 times, most recently from 84dc248 to ed7708b Compare March 14, 2024 11:11
@znapy znapy changed the title gPypath4pylint PR: Add the PYTHONPATH environment from pythonpath-manager to the pylint plugin Mar 14, 2024
@znapy znapy changed the title PR: Add the PYTHONPATH environment from pythonpath-manager to the pylint plugin PR: Add the PYTHONPATH environment from manager to the pylint Mar 14, 2024
@znapy znapy force-pushed the pypath4pylint branch 2 times, most recently from c654d8e to 311e53f Compare March 18, 2024 12:02
@znapy znapy force-pushed the pypath4pylint branch 2 times, most recently from 7dbd860 to 9a07c58 Compare October 15, 2024 16:39
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @znapy! I think this is really good improvement.

I left several suggestions for you to fix minor code style issues here and there to start with.

In case you haven't used Github reviews before, you can submit all those suggestions in a single commit by adding them to the batch, as described here.

spyder/plugins/pylint/main_widget.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/main_widget.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/main_widget.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/main_widget.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/tests/test_pylint.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/tests/test_pylint.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/tests/test_pylint.py Outdated Show resolved Hide resolved
spyder/plugins/pylint/tests/test_pylint.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title PR: Add the PYTHONPATH environment from manager to the pylint PR: Pass paths from the Pythonpath manager to the Pylint plugin Oct 23, 2024
@ccordoba12 ccordoba12 added this to the v6.1.0 milestone Oct 23, 2024
@ccordoba12
Copy link
Member

ccordoba12 commented Oct 23, 2024

Also, so sorry for not paying attention to your work before. We were busy preparing things for Spyder 6.0 and missed your PR. But we'll try to merge it for 6.1, to be released in a few months.

@ccordoba12
Copy link
Member

Thanks for the update @znapy! Overall your changes look good to me and our tests are passing, which is good too.

@dalthviz, could you give this a manual check, following @znapy OP, to see if you get the expected behavior? Thanks!

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 @znapy ! Sorry for the late review! Gave this a local check and seems like things are working as expected 👍

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @znapy! It's really cool improvement.

@ccordoba12 ccordoba12 merged commit a642045 into spyder-ide:master Nov 28, 2024
17 checks passed
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.

4 participants