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

WIP refactor and add tests #23

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

JamesHutchison
Copy link
Owner

@JamesHutchison JamesHutchison commented Aug 25, 2023

This draft PR starts the refactor process. The idea is that we can decouple the logic from SuperAGI. This is done by having logic passed into classes rather than then being based off of the SuperAGI logic.

An example of this is in summary_generator.py. You can see how SummaryGenerator is in a module that no longer imports super_agi. It actually doesn't need to import langchain either - it has some default chat logic that uses it for convenience but really doesn't need it.

There's an issue with how the superagi toolkit is a separate dependency than superagi itself. The way that python works is that if you have a module to import, it'll import the first valid name it comes across in its list of locations to search. There's a known consternation around this when you decide to stuff submodules under the same namespace but install them in different locations. I believe there's a technique to work around this, but it doesn't appear superagi is doing this. As a result, the codespace has superagi in two locations - one is in the library path for the virtual environment, which is looked at first. This contains the toolkit logic. The other path is under /workspaces/SuperAGI and this contains everything else. Python does the first import, then assumes that submodules that are missing in that import nowhere else to be found.

I've seen this issue before when using Google's Python library. Specifically, they had separated out their gRPC logic from app engine and this created problems because you had vendored google libraries but also built-in google libraries.

My suggestion at this time is to just live with this issue because the proper fix, I think, would be in SuperAGI. Since we're doing a refactor that decouples things for our unit tests, we can avoid importing from superagi anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants