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

First-party import incorrectly resolved to site-packages rather than source #7365

Closed
dalcinl opened this issue Aug 27, 2022 · 11 comments · Fixed by pylint-dev/astroid#1756
Closed
Assignees
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code namespace-package Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@dalcinl
Copy link

dalcinl commented Aug 27, 2022

Bug description

Please see the following logs. Note the strange site-packages. prefix in the package/module namespace. I cannot reproduce this issue locally using virtual environments (running via tox) or installing into user site-packages.

Configuration

I'm using the following .pylintrc file.

Command used

pylint mpi4py

After installing the mpi4py package with pip install . on a git checkout.

Pylint output

************* Module site-packages.mpi4py.typing
/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/site-packages/mpi4py/typing.py:15:0: E0611: No name 'Datatype' in module 'site-packages.mpi4py.MPI' (no-name-in-module)
/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/site-packages/mpi4py/typing.py:15:0: E0611: No name 'BottomType' in module 'site-packages.mpi4py.MPI' (no-name-in-module)
/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/site-packages/mpi4py/typing.py:15:0: E0611: No name 'InPlaceType' in module 'site-packages.mpi4py.MPI' (no-name-in-module)
************* Module site-packages.mpi4py.bench
/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/site-packages/mpi4py/bench.py:26:11: I1101: Module 'site-packages.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)

... and so on.

Expected behavior

No output other than the final evaluation.

Pylint version

pylint 2.15.0
astroid 2.12.4
Python 3.10.6

OS / Environment

Ubuntu 22.04 or 20.04 (GitHub Actions runner images)

Additional dependencies

N/A, see logs.

As I mentioned before, I cannot reproduce locally.

Any tips about how to debug this issue within the GitHub Actions runner environment?

@dalcinl dalcinl added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 27, 2022
@Pierre-Sassoulas Pierre-Sassoulas added namespace-package False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 27, 2022
@DanielNoord
Copy link
Collaborator

This has something to do with how we infer the name of the package to lint. I'm not quite sure why we are regressing on this in 2.15. We're actually working on this in astroid right now, but those changes aren't included in 2.15.

I'm not sure if this is a namespace package issue but rather something with the specific way this CI installs the mpi4py package and then tries to lint it.

@jacobtylerwalls
Copy link
Member

I noticed you're invoking flake8 as flake8 src/mpi4py but pylint as pylint mpi4py, which is forcing pylint to look around for an installed package named mpi4py. When I clone the repo without installing it with pip, pylint fails with pylint mpi4py and succeeds with pylint src/mpi4py, which is what I would expect.

Regarding what made the prior behavior work with pip install ., I don't know, as I can't pip install due to wheel problems:

      clang -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -arch x86_64 -g -I/Library/Frameworks/Python.framework/Versions/3.10/include/python3.10 -c _configtest.c -o _configtest.o
      _configtest.c:2:10: fatal error: 'mpi.h' file not found
      #include <mpi.h>
               ^~~~~~~
      1 error generated.
      failure.
      removing: _configtest.c _configtest.o
      error: Cannot compile MPI programs. Check your configuration!!!
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for mpi4py
Failed to build mpi4py
ERROR: Could not build wheels for mpi4py, which is required to install pyproject.toml-based projects

@jacobtylerwalls jacobtylerwalls changed the title Regression on GitHub Actions runners Package under site-packages unexpectedly linted rather than cloned source Aug 27, 2022
@jacobtylerwalls jacobtylerwalls added the Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine label Aug 27, 2022
@DanielNoord
Copy link
Collaborator

@jacobtylerwalls That sounds exactly like I would expect. Well done noticing that!

@jacobtylerwalls jacobtylerwalls added the Waiting on author Indicate that maintainers are waiting for a message of the author label Aug 27, 2022
@dalcinl
Copy link
Author

dalcinl commented Aug 27, 2022

noticed you're invoking flake8 as flake8 src/mpi4py but pylint as pylint mpi4py,

That's on purpose. I've been doing it that way for long time, tools like flake8 wants to run under the actual sources, they do not search for packages, and they do not do introspection of extension modules. pylint does perform introspection but I'm not building in-place, so yes, pylint has to search for the installed package.

which is forcing pylint to look around for an installed package named mpi4py.

yes, indeed.

When I clone the repo without installing it with pip, pylint fails with pylint mpi4py

Expected, the package is not installed, and I use a src layout in my source tree.

and succeeds with pylint src/mpi4py, which is what I would expect.

Yes, but you should get tons of warnings due to the missing C extension module

Regarding what made the prior behavior work with pip install ., I don't know, as I can't pip install due to wheel problems:

Sorry! Yes, mpi4py's extension modules have C header and library dependencies.
For the purpose of linting, you can set the environ vars MPICFG=nompi-fast and CFLAGS=-O0 for a quick build and install (you still need a C compiler and Python development headers and libraries, as usual to build any C ext module). The resulting package will be rather useless at runtime, but pylint should be able to import and introspect things.

In short, you can install the following way:

$ MPICFG=nompi-fast CFLAGS=-O0 pip install .

See GitHub Actions's workflow file right here, you can see that's the way I install the package for linting.

@jacobtylerwalls
Copy link
Member

Thanks for your patience with me. The additional instructions were extremely helpful--I can reproduce locally now. The behavior changed in pylint-dev/astroid@5067f08 as I think other maintainers suspected. I'm having a look now.

@jacobtylerwalls jacobtylerwalls added namespace-package and removed Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine labels Aug 27, 2022
@jacobtylerwalls
Copy link
Member

Something like this should do, I'll work up a PR:

diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py
index f082e9c4..3fa5b1b3 100644
--- a/astroid/interpreter/_import/util.py
+++ b/astroid/interpreter/_import/util.py
@@ -13,6 +13,8 @@ from importlib.util import _find_spec_from_path  # type: ignore[attr-defined]
 
 @lru_cache(maxsize=4096)
 def is_namespace(modname: str) -> bool:
+    from astroid.modutils import EXT_LIB_DIRS
+
     if modname in sys.builtin_module_names:
         return False
 
@@ -70,6 +72,8 @@ def is_namespace(modname: str) -> bool:
 
         # Update last_submodule_search_locations
         if found_spec and found_spec.submodule_search_locations:
+            if any(location in EXT_LIB_DIRS for location in found_spec.submodule_search_locations):
+                return False
             last_submodule_search_locations = found_spec.submodule_search_locations
 
     return (

@jacobtylerwalls jacobtylerwalls self-assigned this Aug 27, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.15.1 milestone Aug 27, 2022
@jacobtylerwalls jacobtylerwalls added Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Waiting on author Indicate that maintainers are waiting for a message of the author labels Aug 27, 2022
@jacobtylerwalls jacobtylerwalls changed the title Package under site-packages unexpectedly linted rather than cloned source First-party import incorrectly resolved to site-packages rather than source Aug 27, 2022
@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Aug 27, 2022

@dalcinl, I've put up a PR at pylint-dev/astroid#1756, so I would be very grateful if you had an opportunity to test the fix. Thanks again for the report!

@dalcinl
Copy link
Author

dalcinl commented Aug 27, 2022

@jacobtylerwalls Where you able to reproduce the issue using either a virtual environment or a user-site install? Could you share the exacts steps you used? As I said before, I was not able to reproduce the issue locally. Testing your PR without being able to reproduce the failure on my side first is not so useful.

@jacobtylerwalls
Copy link
Member

This reproduces for me without a virtual env on MacOS:

git clone https://github.com/mpi4py/mpi4py
cd mpi4py
MPICFG=nompi-fast CFLAGS=-00 pip install .
pip install pylint==2.15.0
pip install astroid==2.12.4
pylint mpi4py

@dalcinl
Copy link
Author

dalcinl commented Aug 28, 2022

I cannot reproduce!! I'm using Homebrew Python 3.10.6 on macOS. Do you have any clue about what could be going on?

@jacobtylerwalls
Copy link
Member

No, sorry. I might try a python.org python.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code namespace-package Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants