python-injector / injector

Python dependency injection framework, inspired by Guice
BSD 3-Clause "New" or "Revised" License
1.3k stars 81 forks source link

[Question] Circular imports and "deferred" #170

Open bnorick opened 3 years ago

bnorick commented 3 years ago

I have an issue with circular imports that I am having trouble resolving.

Here is a simple repro case which is similar to what I am doing:

$ tree
.
└── circular
    ├── a.py
    ├── b.py
    ├── c.py
    └── main.py
1 directory, 4 files

$ cat circular/a.py 
# from circular import b
import circular.b as b

class A:
    def __init__(self):
        print(f'A.__init__ {id(self)=}')

$ cat circular/b.py 
from injector import inject, Injector

import circular.c as c

class B:
    @inject
    def __init__(self, injector: Injector):
        print(f'B.__init__ {id(self)=} {id(injector)=}')

$ cat circular/c.py 
from injector import inject

import circular.b as b

class C:
    @inject
    def __init__(self, b: 'b.B'):
        print(f'C.__init__ {id(self)=} {id(b)=}')

$ cat circular/main.py
# add to path to make this runnable from anywhere
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent.parent))

from injector import Injector

import circular.b as b

def main():
    injector = Injector()
    b_inst = injector.get(b.B)

if __name__ == '__main__':
    main()

When I run this, I get an AttributeError related to circular imports caused by a call to get_type_hints. I hoped this would defer, due to the use of a forward reference in c.py and module imports, but maybe I am misunderstanding?

Traceback (most recent call last):
  File "inj\circular\main.py", line 8, in <module>
    import circular.b as b
  File "inj\circular\b.py", line 3, in <module>
    import circular.c as c
  File "inj\circular\c.py", line 6, in <module>
    class C:
  File "inj\circular\c.py", line 8, in C
    def __init__(self, b: 'b.B'):
  File "[venv]\lib\site-packages\injector\__init__.py", line 1367, in inject
    bindings = _infer_injected_bindings(function, only_explicit_bindings=False)
  File "[venv]\lib\site-packages\injector\__init__.py", line 1176, in _infer_injected_bindings
    bindings = get_type_hints(callable, include_extras=True)
  File "[venv]\lib\site-packages\typing_extensions.py", line 1880, in get_type_hints
    hint = typing.get_type_hints(obj, globalns=globalns, localns=localns)
  File "[python]\Python38\lib\typing.py", line 1261, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "[python]\Python38\lib\typing.py", line 270, in _eval_type
    return t._evaluate(globalns, localns)
  File "[python]\Python38\lib\typing.py", line 518, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
AttributeError: partially initialized module 'circular.b' has no attribute 'B' (most likely due to a circular import)

Since I don't have a full understanding of Injector internals, I tried a longshot of adding AttributeError to the try/except (in addition to NameError that's already caught) in _infer_injected_bindings at line 1164. This actually works for both the simplified repro and my actual application. However, I don't know what other implications this change might have and I wanted to seek you advice.

Thanks!

jstasiak commented 3 years ago

As much I don't really want to encourage people using circular imports I believe you're correct in narrowing down the underlying issue and this seems like a sensible change, I'll happily accept a PR implementing this as long as there's a test provided (will require a test package with a circular import to replicate the situation).