> I think this is a novel way to solve the problem, and I think it could > be a step in the right direction. > > However, I'm not sure we should land it this late in 1.9. It seems > risky to introduce the extra logic now. It seems you are creating > "proxy" objects, and I don't fully understand how all of the proxying > logic works. For example, do the objects returned by the lazy importer > obey "normal" python conventions, such as returning the appropriate > type when isinstance() is called? https://docs.python.org/2/reference/datamodel.html#customizing-instance-and-subclass-checks says that isinstance() and issubclass() are implemented by calling __instancecheck__() and __subclasscheck__() on the type being checked against, i.e.: isintance(thing, TypeA) --> TypeA.__instancecheck__(thing) issubclass(TypeA, TypeB) --> TypeB.__subclasscheck__(TypeA) The Stub instances that this branch deposits override __getattribute__() to forward attribute look-ups to the lazily-loaded object. This method always called, even when accessing preexisting attributes. The similar __getattr__() method is only called for attributes that don't exist. The Stub instances also override __setattr__(), __delattr__(), and __call__() but it's possible these are not actually required. I will try removing them. I will also add isinstance() and issubclass() checks. It's possible that the one thing that these Stub instances won't fool is an `is` check, e.g. `stub is Something`, because `is` is not implemented as a call to a __double_underscore__() method. > > It seems that another way to solve the circular import problem would > be to import modules rather than functions at the top level, so that > the specific dependencies we need aren't resolved until the correct > time. But that would lead to code like: This is a good point. It works up to a point. Subclassing, for example, demands that we dereference names within the module. However, the code in this branch can't help that situation either. We may be able to solve all circular reference problems by only ever importing modules. We might need to avoid code in packages' __init__.py too. This has a few downsides: * Lint checks will catch fewer naming issues; none currently check that a name exists in the target module. * Patching in tests, while not an ideal practice, would have to be done in the owning module rather than in the module under test. This can be problematic (patching too much) but nothing we can't work around. * Readability. It helps to work with short names and we'd lose that. * A small performance penalty, though nothing we ought to care about. None of those are particularly bad. Your perspective has made me realise that lazy-importing is (only?) useful for: (a) Using short names for convenience and readability. (b) Deferring work to increase start-up performance. Now, (b) is irrelevant to us, and (a) might not be a big enough win to justify this branch's code. > > from maasserver.models import fabric > > ... > > def foo(): > f = fabric.Fabric() > # or... > Fabric = fabric.Fabric > f = Fabric() > > ... which seems less than ideal. (but perhaps no worse than the extra > 'import' line we have now, or writing a few hundred lines of > lazy-importing code!) > > Just thought of another thing - this may throw off code introspection > tools (such as IPython, PyCharm, etc), which gives me pause... Agreed, me too. Okay, back-burner for this branch. Thanks for your review. Sorry it took me so long to reply.