Skip to content
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

Improve InvokeAI import patterns #6575

Open
RyanJDick opened this issue Jul 3, 2024 · 0 comments
Open

Improve InvokeAI import patterns #6575

RyanJDick opened this issue Jul 3, 2024 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@RyanJDick
Copy link
Collaborator

RyanJDick commented Jul 3, 2024

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:

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 RyanJDick added the enhancement New feature or request label Jul 3, 2024
@RyanJDick RyanJDick changed the title [enhancement]: Improve InvokeAI import patterns Improve InvokeAI import patterns Jul 3, 2024
@RyanJDick RyanJDick self-assigned this Jul 3, 2024
RyanJDick added a commit that referenced this issue Jul 4, 2024
## Summary

This PR migrates all relative imports to absolute imports, and adds a
ruff check to enforce this going forward.

The justification for this change is here:
#6575

## QA Instructions

Smoke test all common workflows. Most of the relative -> absolute
conversions could be completed automatically, so the risk is relatively
low.

## Merge Plan

As with any far-reaching change like this, it is likely to cause some
merge conflicts with some in-flight branches. Unfortunately, there's no
way around this, but let me know if you can think of in-flight work that
will be significantly disrupted by this.

## Checklist

- [x] _The PR has a short but descriptive title, suitable for a
changelog_
- [x] _Tests added / updated (if applicable)_ N/A
- [x] _Documentation added / updated (if applicable)_ N/A
@RyanJDick RyanJDick mentioned this issue Jul 15, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant