-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
pylint reports false 'import-error' since version 3.3.2 in conjunction with src layout and namespace packages // false-positive #10185
Comments
According to be530855265e6548ce1e5db54b2e3dd95448f45a is the first bad commit
commit be530855265e6548ce1e5db54b2e3dd95448f45a
Author: Julfried <[email protected]>
Date: Sun Oct 27 21:58:11 2024 +0100
Fix source root not recognized (#10036)
doc/whatsnew/fragments/10026.bugfix | 3 +++
pylint/lint/expand_modules.py | 2 +-
tests/pyreverse/test_main.py | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 1 deletion(-)
create mode 100644 doc/whatsnew/fragments/10026.bugfix |
I think there's a usage problem with either your command, or your config file. If you set source root to "src", that would mean that you'd want the namespace to start inside that folder. Just like if you'd cd into src and see namespace.library as importable. If you want to keep your command called as is, and your imports really look like "import src.namespace.library", then in your config it should not have a source root configured like that. For you, it is a regression as you adapted your Pylint call for the non-working source root config option |
Thanks, @echoix for this explanation. I explicitly want the source repo to use the "src" layout according to https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/. To my understanding this is what may minimal example shows. That's the reason why I configured pylint with Inside this source root, there's a namespace package according to PEP420 (without Calling pylint with Should have been the correct way of calling pylint with this configuration If yes, sorry for the confusion and I will adapt my linter script and pylint dependency. |
Yes, that's what I think so. It was impossible to use correctly before, especially if there was more than one level deep, or multiple source roots. (With one of the source roots being used as an installed package in the second one). Pylint had to be called with different working directories and config, and couldn't just know the correct module name for imports. You can call Pylint with files or modules. For modules, you'd want to call it with the same module name as when users import your library in their files. |
Thanks for the clarification, @echoix. Your explanation is spot-on and aligns with my original intent behind the fix in #10036. Before this change, pylint didn’t handle source-roots correctly, especially in cases involving the src layout. Users had to adapt by calling pylint with non-standard paths (e.g., src.namespace.library), which was more of a workaround than correct usage. The goal of my PR was to fix this behavior so that configurations like source-roots = "src" work as expected, allowing users to call pylint namespace.library as it would be imported in actual code. I opened #10173 after comments from @webknjaz, as they described similar challenges with source root handling and implicit namespaces. If this issue proves that the behavior works as described for PEP 420 namespaces, it might also help resolve #10173. @o-moe, could you clarify whether the issue here behaves correctly with implicit namespaces as specified in PEP 420? Let me know if further clarification is needed—I’m happy to assist! |
@echoix If your statement regarding how to call modules with pylint is the desired/expected behavior
Then I can confirm that it was broke before v3.3.2 (without me actually knowing it) and is working since then. Sorry for the confusion/wrong issue. @Julfried To my understanding, the example I provide, uses Python's src layout and has a namespace "namespace" according to PEP420 and from v3.3.2 on, pylint is able to call the library module within the namespace by PEP420 also allows for nested namespaces which I did not use in my example and namespaces should be independent from the layout used (src and flat). Do you want me to verify these scenarios as well? |
Thank you for the clarification, @o-moe! I’m glad to hear the fix aligns with PEP 420 and works as intended from v3.3.2 onward. While the old behavior is no longer supported, it’s worth noting it was technically incorrect. The current implementation aligns with PEP 420, which I think is a step in the right direction :) If you’re able to verify other scenarios like nested namespaces (e.g., namespace.subnamespace.library) or usage in flat vs. src layouts, it would be very helpful for confirming full PEP 420 compliance. |
I extended my minimal example to include nested namespaces according to PEP420 and used Worth noting:
For reference, here's a tree of my minimal example archive attached above: (.venv) bm@mba minimal_pylint_example % tree
.
├── LICENSE
├── README.md
├── pylint.txt
├── pyproject.toml
└── src
└── namespace
└── library
├── __init__.py
├── copyright.py
├── library.py
└── py.typed
4 directories, 8 files
|
Thanks for verifying, @o-moe! Since the behavior now aligns with PEP 420 and works as expected from v3.3.2 onward, I believe this issue can be closed. The previous behavior ( @Pierre-Sassoulas, let me know if you think further clarification is needed, this looks to be resolved. Thanks again for the detailed testing, @o-moe! |
Thank you for investigating everyone ! |
Bug description
Pylint reports (since version 3.3.2) a false
import-error
when used in conjunction with Python's "src" layout and namespace packages. Please see attached minimal repository to reproduce the error. If the same repo with the same config is linted with version 3.3.1, no falseimport-error
occurs.Maybe related to #10147
Configuration
Command used
Pylint output
Expected behavior
No
import-error
minimal_pylint_example.zip
Pylint version
OS / Environment
Windows 10
Additional dependencies
The text was updated successfully, but these errors were encountered: