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

Issues linting project installed in editable mode #8829

Closed
dalcinl opened this issue Jul 7, 2023 · 10 comments
Closed

Issues linting project installed in editable mode #8829

dalcinl opened this issue Jul 7, 2023 · 10 comments
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@dalcinl
Copy link

dalcinl commented Jul 7, 2023

Bug description

After installing my pyproject.toml project in editable mode, running pylint on the project's top level source directory seems to be broken. I experienced similar problems some time ago, see #7365.

Configuration

No response

Command used

TMP=/tmp

# Create venv with up-to-date pip
python -m venv $TMP/venv
source $TMP/venv/bin/activate
python -m pip install --upgrade pip
python -m pip install pylint

# Clone project repository and install in editable mode
git clone --depth 1 https://github.com/mpi4py/mpi4py.git $TMP/mpi4py
env CFLAGS=-O0 MPICFG=nompi-fast python -m pip install --editable $TMP/mpi4py

# Running out of the source tree succeeds with no warnings
mkdir -p $TMP/workdir
cp $TMP/mpi4py/.pylintrc $TMP/workdir 
cd $TMP/workdir
pylint mpi4py

# Running in the source tree fails with many warnings
cd $TMP/mpi4py
pylint mpi4py

Pylint output

************* Module src.mpi4py.typing
src/mpi4py/typing.py:27:0: E0611: No name 'Datatype' in module 'src.mpi4py.MPI' (no-name-in-module)
src/mpi4py/typing.py:27:0: E0611: No name 'BottomType' in module 'src.mpi4py.MPI' (no-name-in-module)
src/mpi4py/typing.py:27:0: E0611: No name 'InPlaceType' in module 'src.mpi4py.MPI' (no-name-in-module)
************* Module src.mpi4py.bench
src/mpi4py/bench.py:26:11: I1101: Module 'src.mpi4py.MPI' has no 'Get_processor_name' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects. (c-extension-no-member)
src/mpi4py/bench.py:77:16: I1101: Module 'src.mpi4py.MPI' has no 'Wtime' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects. (c-extension-no-member)
...

Expected behavior

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Pylint version

pylint 2.17.4
astroid 2.15.5
Python 3.11.4 (main, Jun  7 2023, 00:00:00) [GCC 13.1.1 20230511 (Red Hat 13.1.1-2)]

OS / Environment

Fedora 38

Additional dependencies

No response

@dalcinl dalcinl added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 7, 2023
@DanielNoord
Copy link
Collaborator

Which version of setuptools are you using to install this package?

@dalcinl
Copy link
Author

dalcinl commented Jul 10, 2023

If you look at the instruction in my reproducer, I'm upgrading to the latest pip. My project uses pyproject.toml with requires = ["setuptools >= 42", "build"] in the [build-system] table. Therefore, pip should be using latest setuptools in an isolated build environment that goes away after install. Therefore, I cannot easily confirm an exact version, but it should be the latest available on PyPI.

The more relevant detail I forgot to mention is that the package get installed with the new PEP 660 stuff. This means setuptools is creating a __editable__.<pkg-name-and-version>.pth file in site-packages, and that file contains a single line with the absolute path to the package source directory (as my project is using an src layout, the path is /path/to/project/src, if trying my reproducer above the __editable__.*.pth file in the venv's site-packages should contain a single line with /tmp/mpi4py/src).

@dalcinl
Copy link
Author

dalcinl commented Jul 12, 2023

Which version of setuptools are you using to install this package?

Now I can confirm the build backend is using setuptools 68.0.0 to install the package in editable mode.
The issue is reproducible on macOS Ventura as well.

@DanielNoord
Copy link
Collaborator

I probably won't have time to look into this in the next month due to holidays, sorry!

It should have been fixed with pylint-dev/astroid#1752 but apparently it broke again? Or setuptools changed something agian.
If you want to debug this yourself, looking at what the code in that PR is doing for your own project might be insightful.

@thebalaa
Copy link

thebalaa commented Jul 27, 2023

Hit this issue today in pylint extension with vscode, can confirm the package resolution only fails when the package is installed with -e (editable)

Linking #4057 to help others find this issue

@DanielNoord
Copy link
Collaborator

#8896 gives a reproducer.

@Pierre-Sassoulas Pierre-Sassoulas added Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 4, 2023
@fredizzimo
Copy link

I debugged this a bit, and it looks like pylint-dev/astroid#1752 does not deal with _EditableNamespaceFinder correctly. They are registered in sys.path_hook instead of sys.meta_path and therefore not found.

Furthermore it does not pass the full module path to find_spec

So, an editable package with a namespace package root will not work.

This seems to fix it:

diff --git a/astroid/interpreter/_import/spec.py b/astroid/interpreter/_import/spec.py
index 5a7c4e82..4b90339e 100644
--- a/astroid/interpreter/_import/spec.py
+++ b/astroid/interpreter/_import/spec.py
@@ -390,7 +390,7 @@ def _find_spec_with_path(
         return finder_instance, spec
 
     # Support for custom finders
-    for meta_finder in sys.meta_path:
+    for meta_finder in sys.meta_path + sys.path_hooks:
         # See if we support the customer import hook of the meta_finder
         meta_finder_name = meta_finder.__class__.__name__
         if meta_finder_name not in _MetaPathFinderModuleTypes:
@@ -413,7 +413,8 @@ def _find_spec_with_path(
         if not hasattr(meta_finder, "find_spec"):
             continue
 
-        spec = meta_finder.find_spec(modname, submodule_path)
+        full_name = ".".join(module_parts)
+        spec = meta_finder.find_spec(full_name, submodule_path)
         if spec:
             return (
                 meta_finder,

But I'm not confident that I understand everything properly to send a PR to the astroid repository.

@DanielNoord
Copy link
Collaborator

I debugged this a bit, and it looks like pylint-dev/astroid#1752 does not deal with _EditableNamespaceFinder correctly. They are registered in sys.path_hook instead of sys.meta_path and therefore not found.

Furthermore it does not pass the full module path to find_spec

So, an editable package with a namespace package root will not work.

This seems to fix it:

diff --git a/astroid/interpreter/_import/spec.py b/astroid/interpreter/_import/spec.py
index 5a7c4e82..4b90339e 100644
--- a/astroid/interpreter/_import/spec.py
+++ b/astroid/interpreter/_import/spec.py
@@ -390,7 +390,7 @@ def _find_spec_with_path(
         return finder_instance, spec
 
     # Support for custom finders
-    for meta_finder in sys.meta_path:
+    for meta_finder in sys.meta_path + sys.path_hooks:
         # See if we support the customer import hook of the meta_finder
         meta_finder_name = meta_finder.__class__.__name__
         if meta_finder_name not in _MetaPathFinderModuleTypes:
@@ -413,7 +413,8 @@ def _find_spec_with_path(
         if not hasattr(meta_finder, "find_spec"):
             continue
 
-        spec = meta_finder.find_spec(modname, submodule_path)
+        full_name = ".".join(module_parts)
+        spec = meta_finder.find_spec(full_name, submodule_path)
         if spec:
             return (
                 meta_finder,

But I'm not confident that I understand everything properly to send a PR to the astroid repository.

Please do send a PR! I can help with reviewing (and will).

Nobody understands this (clearly) but any help or collaboration will bring us closer to doing so 😄

@dalcinl
Copy link
Author

dalcinl commented May 23, 2024

After the release of pylint 3.2.2 and astroid 3.2.2, the issue seems to be fixed.

@dalcinl dalcinl closed this as completed May 23, 2024
@Pierre-Sassoulas
Copy link
Member

Thank you for triaging @dalcinl !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

5 participants