add `config` as a option parameter of Interpreter __init__ function
Describe the changes you have made:
I have introduced config as a optional parameter in the Interpreter's __init__ function. This change was necessary to eliminate confusion that arose when an Interpreter instance was initialized with system_message and model among others, yet upon reviewing the source code, it appeared that nothing was passed into the __init__ function.
Reference any relevant issue (Fixes #000)
None
- [x] I have performed a self-review of my code:
I have tested the code on the following OS:
- [ ] Windows
- [x] MacOS
- [x] Linux
AI Language Model (if applicable)
- [] GPT4
- [] GPT3
- [] Llama 7B
- [] Llama 13B
- [] Llama 34B
- [] Huggingface model (Please specify which one)
Hi @DayuanJiang, thanks for opening this PR! Can you let me know what a workflow that uses this might look like? We have an instance initialized automatically by importing interpreter, so I don't think we ever use __init__ but i could be wrong.
This is my usual workflow:
import interpreter
interpreter.api_key = "<key>"
interpreter.chat()
Is there a workflow you have that deals with the class itself?
@KillianLucas I appreciate your response. My intention with this PR is to make the code more intuitive and easier to understand for anyone who might be exploring the internal behavior of the Interpreter. It is just a minor fix and doesn't impact the current usage in any way.
I found it somewhat counterintuitive that a class was initialized using an external config, yet the config was not set as a parameter of the __init__ function.
This observation stems from my curiosity about the internal workings of the Open Interpreter. Upon reviewing the source code of the Interpreter, I noticed that nothing was passed into the __init__ function, despite the system_message being set to a lengthy string. (The config = get_config() line at the end of the __init__ function was something I initially overlooked.)
I think you're right and I think we should merge this. To go even further, honestly it feels like we should even move get_config out of the class and into the cli.py.
That way your settings, accessed by running --config, would only apply to terminal usage.
The Python object would be more "stateless" -- by initializing with config = None it would default to the "official" open interpreter settings. That feels more predictable to me. What do you think?
@KillianLucas
If get_config is moved out of the class, we also need to change interpreter/__init__.py to initialize the class with:
config = get_config()
sys.modules["interpreter"] = Interpreter(config)
Regardless, I believe whether to move it from the class or not is simply a matter of preference. Both options are acceptable.
Ended up actually going with something like this! The new Python version takes a million parameters, everything config here would take. Then config happens around the initialization of that object. Will merge in favor of that new method, but your approach was 100% correct here.