griptape-ai / griptape

Modular Python framework for AI agents and workflows with chain-of-thought reasoning, tools, and memory.
https://www.griptape.ai
Apache License 2.0
2.02k stars 170 forks source link

Removed the `__all__` declaration from the `griptape.mixins` module. #1164

Closed collindutter closed 2 months ago

collindutter commented 2 months ago

Describe your changes

Griptape often uses __all__ in its packages to simplify module imports for users. However, this convenience comes with a tradeoff: all modules within the package must be imported to be included in __all__. While this is usually not an issue, it can lead to circular dependency problems in certain cases.

The griptape.mixins package is particularly prone to these issues because its modules aren't restricted to specific parts of the framework. As a result, mixin modules can easily introduce circular dependencies. For instance, simply adding a J2 import to Rule can trigger the following error:

ImportError while loading conftest '/Users/collindutter/Documents/griptape/griptape/tests/unit/conftest.py'.
tests/unit/conftest.py:3: in <module>
    from tests.mocks.mock_drivers_config import MockDriversConfig
tests/mocks/mock_drivers_config.py:3: in <module>
    from griptape.configs.drivers import DriversConfig
griptape/configs/__init__.py:1: in <module>
    from .base_config import BaseConfig
griptape/configs/base_config.py:7: in <module>
    from griptape.mixins.serializable_mixin import SerializableMixin
griptape/mixins/__init__.py:4: in <module>
    from .rule_mixin import RuleMixin
griptape/mixins/rule_mixin.py:7: in <module>
    from griptape.rules import Rule, Ruleset
griptape/rules/__init__.py:1: in <module>
    from griptape.rules.rule import Rule
griptape/rules/rule.py:5: in <module>
    from griptape.utils.j2 import J2
griptape/utils/__init__.py:6: in <module>
    from .command_runner import CommandRunner
griptape/utils/command_runner.py:5: in <module>
    from griptape.artifacts import BaseArtifact, ErrorArtifact, TextArtifact
griptape/artifacts/__init__.py:1: in <module>
    from .base_artifact import BaseArtifact
griptape/artifacts/base_artifact.py:10: in <module>
    from griptape.mixins import SerializableMixin
E   ImportError: cannot import name 'SerializableMixin' from partially initialized module 'griptape.mixins' (most likely due to a circular import) (/Users/collindutter/Documents/griptape/griptape/griptape/mixins/__init__.py)

This PR addresses the issue by removing __all__ from griptape.mixins and updating all imports to reference the specific module directly. While this change is technically breaking, I haven't included a migration guide since most users are unlikely to use mixins directly.

Issue ticket number and link

NA

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:loudspeaker: Thoughts on this report? Let us know!