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

false import-errors with pip >= 21.3 #7306

Closed
belm0 opened this issue Aug 15, 2022 · 23 comments
Closed

false import-errors with pip >= 21.3 #7306

belm0 opened this issue Aug 15, 2022 · 23 comments
Labels
Bug 🪲 Import system Needs astroid update Needs an astroid update (probably a release too) before being mergable

Comments

@belm0
Copy link
Contributor

belm0 commented Aug 15, 2022

Bug description

summary:

  • pylint incorrectly reports import-error with recent pip versions (>= 21.3)
  • the environment uses pip install -e ., on a project with setup.py and pyproject.toml
  • confirmed normal runtime doesn't have import errors, and pylint has no errors with pip < 21.3
  • import of pylint plugins local to the project is also affected by the bug

The following change from pip 21.3.0 may be relevant:

Support editable installs for projects that have a pyproject.toml and use a build backend that supports PEP 660 (pypa/pip#8212).

create project & env:

echo 'import setuptools; setuptools.setup(name="my_project", packages=["foo"])' > setup.py
touch pyproject.toml  # toml must exist to reproduce the problem
mkdir foo scripts
touch foo/__init__.py
echo 'import foo' > scripts/bar.py

python -m venv .venv
source .venv/bin/activate
pip install --upgrade pip pylint
pip install -e .

Configuration

No response

Command used

pylint scripts/bar.py

Pylint output

************* Module bar
scripts/bar.py:2:0: E0401: Unable to import 'foo' (import-error)

Expected behavior

no import-error, as with earlier pip versions

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.8.12 (default, Sep 23 2021, 08:42:37)
[Clang 12.0.0 (clang-1200.0.32.29)]

OS / Environment

No response

Additional dependencies

pip => 21.3

@belm0 belm0 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 15, 2022
@jooola
Copy link

jooola commented Aug 21, 2022

I am having a similar error, but on my side it happens when using setuptools >=64.0.0 which was introduced by https://togithub.com/pypa/setuptools/pull/3488. Have you tried pinning the setuptools version below 64.0.0 for the build backend ?

Here is my failing workflow: https://github.com/libretime/libretime/runs/7938963601?check_suite_focus=true

@jooola
Copy link

jooola commented Aug 22, 2022

Here is another minimal reproducible repository https://github.com/jooola/pylint-import-error-bug

@DanielNoord
Copy link
Collaborator

@jooola Can you confirm that for the reproducing repository doing pip install setuptools==63 and then reinstalling the editable packages fixes the issue? I'm having trouble reproducing the successful behaviour on lower setuptools versions locally.

@jooola
Copy link

jooola commented Aug 23, 2022

@DanielNoord I was was able to fix my issue buy applying the patch I just pushed. Which pin the setuptools version below 64.0.0

https://github.com/jooola/pylint-import-error-bug/blob/main/before_setuptools_v64.0.0.patch

Ooh wait, I had to deactivate and reactivate the virtual environment once everything was installed to get this to work again. I'll try the same with setuptools>=64.0.0.

@jooola
Copy link

jooola commented Aug 23, 2022

Ok, I can confirm this again, setuptools<64 fixes the bug, without necessarily reentering the virtual venv.

@DanielNoord
Copy link
Collaborator

DanielNoord commented Aug 23, 2022

The issue is in the following piece of code:
https://github.com/PyCQA/astroid/blob/d033fe2f4c575d45a1706c52988dbcdf50645c5c/astroid/interpreter/_import/spec.py#L135-L145

On setuptools<64 we successfully find the shared_lib package via sys.path and looking for its __init__.py file.
On setuptools>65 this no longer works. I think this is because editable installs are no longer put on sys.path but I'm note 100% sure. It is definitely due to the setuptools changes though.

The repository with the pinning of setuptools reproduces the issues consistently.

I'm not sure I'll be able to get to this this week so if anybody is interested please go ahead and investigate more or propose a solution. I'm inclined to open an issue against setuptools to ask them how they think we should discover editable packages with the new backend.

Edit: Replied to pypa/setuptools#3518. There seem to be issues with the new system and other static analysis tools. Let's hope they get back to us!

@DanielNoord
Copy link
Collaborator

I have a fix ready for this in pylint-dev/astroid#1752.

However, I'm not sure this is the right way forward. Other linters have been hesitant to add support for import hooks, see:
https://mail.python.org/archives/list/[email protected]/thread/IIVBPYDZR5T5BGPAWFVYS5ZPYDXGVHQN/#OSWHT5VSRGKPSPYD7PQWR2M4OCSL5WO3

I'm not sure we should start supporting them, but at least this works I guess? 😅

@DanielNoord DanielNoord added Bug 🪲 Import system Needs astroid update Needs an astroid update (probably a release too) before being mergable and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 23, 2022
@MiltiadisKoutsokeras
Copy link

pylint is used in our workflows and currently it breaks them with import errors from an editable installation of our projects. Is there something we can change in the usage of pylint to avoid these errors without changing the project's code or file hierarchy?

@DanielNoord
Copy link
Collaborator

DanielNoord commented Sep 2, 2022

It's a bit of a stretch but you could install astroid from my branch in pylint-dev/astroid#1752.

See https://stackoverflow.com/questions/20101834/pip-install-from-git-repo-branch
Or git clone my branch and then install locally.

Fixing stuff in our import system is difficult as much can be broken. So it takes some time for these fixes to make it to release. This branch has been working for me with the new setuptools versions though.

Edit: Or any of the suggestions by @jooola

@jooola
Copy link

jooola commented Sep 2, 2022

pylint is used in our workflows and currently it breaks them with import errors from an editable installation of our projects. Is there something we can change in the usage of pylint to avoid these errors without changing the project's code or file hierarchy?

You can either pin the setuptools version (see the bug reproduction repository) or add the legacy editable option (using the SETUPTOOLS_ENABLE_FEATURES="legacy-editable" env variable) when installing your project.

@MiltiadisKoutsokeras
Copy link

Thank you for your recommendations but the issue is reproduced with these versions:

Python 3.9.2
pip 22.2.2
setuptools 44.1.1

So setuptools is less than 64.

@DanielNoord
Copy link
Collaborator

Even then, installing my local branch should fix this as it add support for custom import hooks.

Also note that it's quite easy to have pip use a different version of setup tools than what's showing up with pip list. I had similar issues while working on the fix.

@MiltiadisKoutsokeras
Copy link

Can you please elaborate on the setuptools version? Is there a way to check if there is inconsistency compared to what pip list shows?

@MiltiadisKoutsokeras
Copy link

The only safe and non-intrusive fix for now is export SETUPTOOLS_ENABLE_FEATURES="legacy-editable" in the Bash script that prepares the Python venv and installs the editable version. Since this is temporary and going to be eventually deprecated I hope there will be a more efficient solution to support the new editable install format.

@Pierre-Sassoulas
Copy link
Member

It seems it might be a bug caused by pypa/setuptools#3499 that affects setuptools 64.0.0 but was fixed in 64.0.1 ?

@niander
Copy link

niander commented Nov 9, 2022

For me, the only way of making pylint to work agian in editable installs is to also use SETUPTOOLS_ENABLE_FEATURES="legacy-editable". I tried lowering the version of setuptools to 63.4.3 but that alone did not fix it. I wonder if the actual issue is indeed on pip. Does anyone know how we can be sure to know which setuptools version pip is using?

@niander
Copy link

niander commented Nov 9, 2022

Ok, so I was able to confirm that setuptools<63.0.0 seems to fix the issue. @DanielNoord I think I know why sometimes it is hard to know which "setuptools" pip is using. I found out that doesn't matter which version I have installed in my venv. Because I have pyproject.toml (I also have setup.cfg and setup.py), the newer versions of pip seems to always load pyproject.toml. And, because I added to the file requires = ["setuptools>=61.0.0", "wheel"], pip seems to install the latest "setuptools" and don't use the one already installed.

I changed my pyproject.toml to requires = ["setuptools>=61.0.0,<63.0.0", "wheel"] and that itself fixed the issue.
Also, adding -v to pip installation shows which build packages pip is installed (or loading) temporarily to install the package.

mkoura added a commit to IntersectMBO/cardano-node-tests that referenced this issue Dec 30, 2022
mkoura added a commit to input-output-hk/cardano-clusterlib-py that referenced this issue Dec 30, 2022
mkoura added a commit to input-output-hk/cardano-clusterlib-py that referenced this issue Dec 30, 2022
@ssbarnea
Copy link
Contributor

ssbarnea commented Feb 7, 2023

I see that this bug happens even with the latest setuptools==67.2.0 and I do not understand why we still consider setuptools being the cause when python imports do really work, is just pylint that thinks that they do not.

Any pinning of setuptools would produce other problems, even if done only for development.

@DanielNoord
Copy link
Collaborator

I don't understand your comment. There is an incompatibility with setuptools and pylint above a certain version.
Nobody is "blaming" setuptools we are just identifying the source of the issue.

Anyway, @Pierre-Sassoulas would you mind releasing astroid. I think this is a fix worth bringing out and we can start migrating the Uninferable stuff in pylint.

@Pierre-Sassoulas
Copy link
Member

Sure let's release a new version of astroid.

@noah-weingarden
Copy link

noah-weingarden commented Feb 19, 2023

@DanielNoord Thanks for your work on pylint-dev/astroid#1752! I installed astroid from its main branch on GitHub so that I could give the fix a try. I'm not sure if I'm doing this too early or not, but you mentioned that you'd like feedback here, so I'll give it a shot. Let me know if I should make a new issue rather than adding it on here.

I'm finding that whereas before pylint reported import-error for modules imported from an editable install, it now correctly identifies the modules and objects available in them like classes, but not submodules. I created a minimal reproducible example with steps to reproduce in this repo. After running pylint tests, I get this output:

************* Module test_everything
tests/test_everything.py:6:14: E1101: Module 'pylint_bug_mre' has no 'goodbye' member; maybe 'Goodbye'? (no-member)
tests/test_everything.py:11:12: E1101: Module 'pylint_bug_mre' has no 'hello' member; maybe 'Hello'? (no-member)

It seems like pylint recognizes the module being imported and the two classes that it exports, but it doesn't recognize its submodules. I confirmed that pylint reports no errors if I use a setup.py instead of pyproject.toml (thereby using the old editable install method instead of the PEP 660 one).

Edit: I dug through astroid's source code and think I've figured out the problem. It's in this piece of code: https://github.com/PyCQA/astroid/blob/ade4c68d86f30d58d35e3eaf4c530af780bbf7e2/astroid/interpreter/_import/spec.py#L450-L460

When you try importing pylint_bug_mre.goodbye, astroid is correctly able to find the first part of the path using the changes that were added in pylint-dev/astroid#1752. After the first iteration of the loop, however, modname is just goodbye, and astroid has no idea where to look for the goodbye module. I've thought of a solution and made a PR here.

@belm0
Copy link
Contributor Author

belm0 commented Oct 30, 2023

Seems to be resolved in pylint 3.0

@Pierre-Sassoulas
Copy link
Member

Let's close then, please open a new issue referencing this one if you still encounter the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Import system Needs astroid update Needs an astroid update (probably a release too) before being mergable
Projects
None yet
Development

No branches or pull requests

8 participants