seddonym / import-linter

Import Linter allows you to define and enforce rules for the internal and external imports within your Python project.
https://import-linter.readthedocs.io/
BSD 2-Clause "Simplified" License
664 stars 45 forks source link

Modules in packages without an `__init__.py` file are ignored #202

Closed neilmacintyre closed 10 months ago

neilmacintyre commented 10 months ago

I have the following project structure:

├── english_class
│   ├── __init__.py
│   ├── subdir
│   │   └── sub_class.py

english_class.subdir.sub_class.py imports math like this:

import math
CONSTANT=(math.tan(0.5))

and english_class.__init__.py imports english_class.subdir.sub_class.py like this:

from english_class.namespace1.sub_class import CONSTANT

When I define the following contract to forbid english_class from importing math , lint-imports passes, even though when I run python -X importtime -c "import english_class" math is displayed as being imported. Is this the expected behaviour? If it is, is there away to configure import-linter to check that contracts are not broken by imports from a sub directory?

[importlinter]
root_packages=
   english_class
include_external_packages=True

[importlinter:contract:1]
name=No math
type=forbidden
source_modules=
   english_class
forbidden_modules=
    math

(In this case the simple fix is adding __init__.py to subdir however for a larger project I am working on sub-directories are being used to organize files within modules and it becomes quite hard to actually enforce the contracts since they can easily be missed by moving a python file into a subdirectory w/o an __init__.py)

seddonym commented 10 months ago

Thanks for the issue!

Because there is no __init__.py file in that subdir, Import Linter isn't viewing it as part of the english_class package.

Import Linter does support namespace packages (see docs) but in my opinion this isn't really a namespace package - there should be an __init__.py file in there and the solution is to include it.

On the larger codebase, is there any reason why you aren't including __init__.py files? If you concerned about accidentally missing them then I would suggest linting for that too, using a different tool, such as flake8-no-pep420.

Does that make sense?

neilmacintyre commented 10 months ago

Thanks! Yeah I was primarily worried about it being easy to miss an __init__.py file but adding linting to ban pep420 resolves that.

One issue that I do see with adding __init__.py, is that if __init__.py imports a forbidden model (or imports a modules that imports a forbidden model) when I import another module in that package, the forbidden model will be imported but import-linter does not detect it. I'm closing this issue and open a new one up for this here: https://github.com/seddonym/import-linter/issues/203