InvokeAI icon indicating copy to clipboard operation
InvokeAI copied to clipboard

Improve InvokeAI import patterns

Open RyanJDick opened this issue 1 year ago • 0 comments

Problem

The InvokeAI import graph is a tangled web ripe with circular imports and import-order dependencies. We currently work around these issues with short-term hacks, but they are a hindrance to development. The good news is that these issues are generally easy to avoid by following some simple patterns.

Examples of workarounds for import issues:

  • https://github.com/invoke-ai/InvokeAI/blob/bb6ff4cf37db3d0dac2c260e662c0009c8b27bdb/invokeai/app/services/config/config_default.py#L159
  • https://github.com/invoke-ai/InvokeAI/blob/bb6ff4cf37db3d0dac2c260e662c0009c8b27bdb/invokeai/backend/ip_adapter/ip_adapter.py#L140-L141
  • https://github.com/invoke-ai/InvokeAI/blob/bb6ff4cf37db3d0dac2c260e662c0009c8b27bdb/invokeai/backend/raw_model.py#L3-L7
  • https://github.com/invoke-ai/invoke-training/blob/main/src/invoke_training/model_merge/scripts/merge_lora_into_model.py#L10-L14

Proposal

Here is a description of the direction that I think we should be moving to solve these problems (discussed offline with several members of the Invoke team):

  1. Cross-directory imports should be one-directional. For example, files in invokeai/app/invocations are expected to import from invokeai.backend, but files in invokeai/backend should not import from invokeai.app.invocations.
  2. Avoid re-exporting symbols from __init__.py files unnecessarily. This pattern makes sense for building clean public APIs, but most of our codebase is non-public (with the exception of invokeai.invocation_api). Also, re-exporting from __init__.py tends to lead to eager importing of modules before they are needed, which exacerbates circular import issues if the import tree is poorly structured, and could be contributing to the slow app launch times.
  3. We have a bad habit of doing stuff on import. During import, we should only be defining classes/functions/constants - not executing code. This anti-pattern has led to several import-order dependencies.
  4. Use absolute imports rather than relative imports. This is primarily a style preference, but PEP 8 recommends absolute imports, and in my experience they work better with Python LSP services (e.g. for auto-import and refactoring features). We currently have a mix of absolute and relative imports (mostly absolute) - we should standardize this. Also, if we enforce absolute imports with a linter, this makes refactoring easier. With absolute imports, if I move invokeai.app.service.some.module, then I can confidently search and replace all imports of this module. With relative imports, it is harder to find all references to the moved module.

Next Steps

This is intended to be a long-lived issue as we gradually work towards the proposed goal state over time. We won't try to solve all of these problem in a single pass.

RyanJDick avatar Jul 03 '24 21:07 RyanJDick