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

ImportError for implicit namespace (pyproject, setuptools) #7875

Open
b-kamphorst opened this issue Nov 30, 2022 · 15 comments
Open

ImportError for implicit namespace (pyproject, setuptools) #7875

b-kamphorst opened this issue Nov 30, 2022 · 15 comments
Labels
High effort 🏋 Difficult solution or problem to solve namespace-package Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed

Comments

@b-kamphorst
Copy link

b-kamphorst commented Nov 30, 2022

Bug description

Hi all,

I try to run pylint on an implicit namespace that is build from a pyproject.toml with setuptools as backend. However, pylint is not able to deduce the namespace and throws an ImportError.

Project lay-out:

|-- pyproject.toml
|-- src
     |-- foo
          |-- bar
               |-- __init__.py
               |-- fun.py

pyproject.toml

[build-system]
requires = ["setuptools", "setuptools-scm"]
build-backend = "setuptools.build_meta"

[project]
name = "foo.bar"
version = "0.0.1"

fun.py

def foo() -> None:
    pass

Configuration

No response

Command used

pip install .
pylint foo.bar

Pylint output

Traceback (most recent call last):
  File "/home/bart/git/pylint-tester/.venv/bin/pylint", line 8, in <module>
    sys.exit(run_pylint())
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/__init__.py", line 35, in run_pylint
    PylintRun(argv or sys.argv[1:])
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/run.py", line 207, in __init__
    linter.check(args)
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 696, in check
    ast_per_fileitem = self._get_asts(fileitems, data)
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 707, in _get_asts
    for fileitem in fileitems:
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 874, in _iterate_file_descrs
    for descr in self._expand_files(files_or_modules).values():
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 881, in _expand_files
    result, errors = expand_modules(
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/expand_modules.py", line 149, in expand_modules
    modpath = _modpath_from_file(
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/pylint/lint/expand_modules.py", line 21, in _modpath_from_file
    return modutils.modpath_from_file_with_callback(
  File "/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/astroid/modutils.py", line 303, in modpath_from_file_with_callback
    raise ImportError(
ImportError: Unable to find module for /home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages/foo/bar/fun.py in /home/bart/git/pylint-tester, 
/home/bart/git/pylint-tester/.venv/bin, 
/home/bart/.pyenv/versions/3.10.8/lib/python310.zip, 
/home/bart/.pyenv/versions/3.10.8/lib/python3.10, 
/home/bart/.pyenv/versions/3.10.8/lib/python3.10/lib-dynload, 
/home/bart/git/pylint-tester/.venv/lib/python3.10/site-packages

Expected behavior

pylint analysis of the module, similar to the following output (obtained by putting fun.py in foo and updating the package name):

************* Module foo
.venv/lib/python3.10/site-packages/foo/__init__.py:1:0: C0104: Disallowed name "foo" (disallowed-name)
************* Module foo.fun
.venv/lib/python3.10/site-packages/foo/fun.py:1:0: C0114: Missing module docstring (missing-module-docstring)
.venv/lib/python3.10/site-packages/foo/fun.py:1:0: C0116: Missing function or method docstring (missing-function-docstring)
.venv/lib/python3.10/site-packages/foo/fun.py:1:0: C0104: Disallowed name "foo" (disallowed-name)

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

Pylint version

pylint 2.15.7
astroid 2.12.13
Python 3.10.8 (main, Nov 30 2022, 11:20:47) [GCC 9.4.0]

OS / Environment

Ubuntu 20.04.5 LTS (in WSL 2 on Windows 10)

Additional dependencies

No response

@b-kamphorst b-kamphorst added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 30, 2022
@b-kamphorst
Copy link
Author

I expect that the issue is somewhere in astroid, specifically in astroid.interpreter._import.util.py:is_namespace, but that code is quite involved. Looking forward to your thoughts!

@b-kamphorst
Copy link
Author

Hi all (special to @jacobtylerwalls)! I know these are busy times, but I feel that this is quite a fundamental issue. I may be making a mistake -- I hope so and please do help me to sort it out! If not, however, you may want to triage this and get a discussion going.

I appreciate your efforts :)

@DanielNoord
Copy link
Collaborator

Which version of setuptools are you using?

@b-kamphorst
Copy link
Author

I've tested this setup (and repreduced the issue) with versions 63.2.0, 65.5.1, and 65.6.3 (latest). I just ensured that pip is also the latest version (22.3.1).

@DanielNoord
Copy link
Collaborator

Could you try with setuptools 60? We have seen issues with recent versions of setuptools.

@b-kamphorst
Copy link
Author

I got the same results with versions 54.2.0, 59.8.0, 60.0.0 and 60.10.0.

@DanielNoord
Copy link
Collaborator

I really want to look into this, but haven't found the time. Sorry! Trying to do so in the coming weeks.

@b-kamphorst
Copy link
Author

@DanielNoord thank you for the heads up -- I appreciate the notification.

I've tried some minor things that did not help (but might we worth noting).

  • [observation] In contrast to my previous configuration with setup.py, this configuration does not create namespace_packages.txt in the bdist / sdist.
  • [experiment] Adding a section [tool.setuptools] and defining namespace_packages = ["foo"] did create the namespace_packages.txt, but it did not seem to matter.
  • [experiment] Finally, I duplicated the project and updated the configuration so that it provides a package foo.bar_2. Installing that package additionally (so that the foo namespace is populated by multiple projects) did not seem to matter (as is expected).

So.. no luck thus far. I look forward to your analysis. Until that time though, let's enjoy the last bit of 2022 and we'll be in touch next year!

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Dec 30, 2022

Thanks for your patience. I just found some time to have a look. This looks like a consequence of pylint-dev/astroid#1756. EDIT: that PR just restored the state of play in astroid before 2.12.0.

The following two diffs (one for astroid, one for pylint) is horribly unreadable in its current state, but it's a quick hack to pass your test case. The trouble is we have two different is_namespace utilities floating around.

EDIT: removed irrelevant proposed diffs


Would I be able to interest you in putting together a pair of PRs that improves these diffs (and adds a test?)


For now, as a workaround, you could lint src.foo.bar, but that may not be acceptable for the same reasons as the OP in #7365.

@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 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Dec 30, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.16.0 milestone Dec 30, 2022
@jacobtylerwalls
Copy link
Member

Actually, looking closer, all the diff really does is revert pylint-dev/astroid#1756. I'm not yet sure of the correct behavior.

@jacobtylerwalls jacobtylerwalls added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed 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 labels Dec 30, 2022
@b-kamphorst
Copy link
Author

@jacobtylerwalls thank you for joining the discussion and providing suggestions!

I'm somewhat surprised by the comment

But immediately return False if we can detect we are in stdlib or external lib (e.g site-packages)

that was introduced in pylint-dev/astroid#1756. In both my local development environment (using venv as displayed in OP) as well as in the gitlab ci (path /usr/local/lib/python3.10/site-packages/foo/...), site-packages is part of the path. Actually my main reason for migrating to a src-layout is to get rid of the confusion between source files and the installed version of a package (I regularly have mypy issues in a flat-layout). But I then might just be confused as I am new to introspection logic.

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Dec 31, 2022

You have a nice test case here. Something should be done to improve the situation. Unfortunately this has been missing from pylint for forever (doesn't look to be recently caused in astroid*). Some of the variations on the failure were also referenced in #1361 before we closed it.

1. pylint foo: silent failure

2. pylint foo.bar: uncaught ImportError

3. pylint foo.bar.fun: resolves to site-packages, violating the principle of #7365, if that's even the correct behavior

(* in astroid 2.12.0, 2. briefly behaved like 3., but we got several bug reports and reverted it in pylint-dev/astroid#1756 in 2.12.5)

Workaround is to use pylint src.foo.bar like flake8. Sorry we don't have a solution at the moment!

@jacobtylerwalls jacobtylerwalls added Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Dec 31, 2022
@jacobtylerwalls jacobtylerwalls removed this from the 2.16.0 milestone Dec 31, 2022
@jacobtylerwalls jacobtylerwalls added the High effort 🏋 Difficult solution or problem to solve label Dec 31, 2022
@b-kamphorst
Copy link
Author

@jacobtylerwalls could you point me to some of these issues to better understand the issue with resolving to site-packages? I don't quite understand what happens in #7365 -- that case seems weird also just because the pylint output states the module prepended with site-packages. I wonder, if both cases have their uses, whether it might be nice to allow both pytest foo and pytest .foo or something to indicate site-packages versus CWD should be leading (absolute vs relative import), or have some kind of flag to allow / prevent either from being used.

I only specified my use case partially. Ultimately, I want to be able to install several namespace packages, all of them part of the same namespace, and then run pylint on the package as a whole (so not just foo.bar, but also foo.bar_2, foo.bar_3, etc. I don't think a pylint src.foo.bar-like solution can do this for me?

@jacobtylerwalls
Copy link
Member

Sure thing.

(* in astroid 2.12.0, 2. briefly behaved like 3., but we got several bug reports and reverted it in pylint-dev/astroid#1756 in 2.12.5)

In addition to #7365 we also got a bug report after the fact in pylint-dev/pylint-plugin-utils#27 (comment).

I agree that the handling of namespace packages needs improving. The code paths through this part of astroid and pylint are terribly tangled. We would just like to find incremental improvements that don't create breaking changes (unless we do really need to advertise breaking changes in a 3.0 version.) Any analysis or draft patches are much appreciated.

@hmc-cs-mdrissi
Copy link
Contributor

I'd expect a solution to this issue to likely give a way of handling this one as well. I did make a pr for it in past and mostly stalled out on writing tests for it/whether that option was worth it. Maybe now I should revive that pr as still feels like any solution that involves guessing sys.path/reverse engineering import system is likely to be tricky. The original pr was this one.

Most of complexity/bugs with namespace packages comes from pylint trying to be smart and guessing what to add to sys.path if packages are uninstalled. This smartness is convenient for simple projects and allowing users to not need to install there own code, but becomes tricky for more complex packages (mostly namespace weirdness). If pylint expected (with an opt-in flag) that all packages must be installed already and doesn't alter sys.path at all, the complexity disappears.

torotil added a commit to moreonion/impact-stack-rest that referenced this issue Oct 2, 2023
This is to accomodate pylint not working with implicit namespace
packages.

See pylint-dev/pylint#7875 or one of its many
duplicates.

isort’s auto-detection seems to need some extra config to recognize the package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High effort 🏋 Difficult solution or problem to solve namespace-package Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed
Projects
None yet
Development

No branches or pull requests

4 participants