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

Fix: NameError: name 'computer' is not defined #937

Merged

Conversation

MikeBirdTech
Copy link
Collaborator

@MikeBirdTech MikeBirdTech commented Jan 17, 2024

Describe the changes you have made:

The run method in the Terminal class is a generator function that executes some code and yields the results one at a time. In a previous commit, the for loop was removed that was used to iterate over these results, which caused the run method to not actually be executed because its results were not being consumed. This commit adds the for loop back to ensure that the run method is properly executed.

Reference any relevant issues (e.g. "Fixes #000"):

#875

Pre-Submission Checklist (optional but appreciated):

  • I have included relevant documentation updates (stored in /docs)
  • I have read docs/CONTRIBUTING.md
  • I have read docs/ROADMAP.md

OS Tests (optional but appreciated):

  • Tested on Windows
  • Tested on MacOS
  • Tested on Linux

@MikeBirdTech MikeBirdTech requested review from KillianLucas and removed request for KillianLucas January 17, 2024 17:52
@MikeBirdTech MikeBirdTech marked this pull request as draft January 17, 2024 18:00
@Notnaton
Copy link
Collaborator

Traceback (most recent call last):
File "", line 1, in
File "E:\dev\open-interpreter\interpreter\core\core.py", line 46, in start_terminal_interface
start_terminal_interface(self)
File "E:\dev\open-interpreter\interpreter\terminal_interface\start_terminal_interface.py", line 543, in start_terminal_interface
for _ in interpreter.computer.run(
File "E:\dev\open-interpreter\interpreter\core\computer\terminal\terminal.py", line 40, in run
for chunk in self._streaming_chat(language, code, stream=True):
^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Terminal' object has no attribute '_streaming_chat'

@Notnaton
Copy link
Collaborator

Added a quick fix, just putting stream=True

@MikeBirdTech MikeBirdTech marked this pull request as ready for review January 18, 2024 02:31
interpreter.computer.run(
"python",
"import time\nfrom interpreter import interpreter\ncomputer = interpreter.computer", # We ask it to use time, so
for _ in interpreter.computer.run(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually why I added that part that pulls from the stream! It allows you to just run interpreter.computer.run without the for _ in syntax. I'll merge but will revert this.

@@ -630,7 +631,7 @@ def start_terminal_interface(interpreter):

# Set attributes on interpreter
for argument_name, argument_value in vars(args).items():
if argument_value != None:
if argument_value is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://peps.python.org/pep-0008/#programming-recommendations

"Comparisons to singletons like None should always be done with is or is not, never the equality operators."

@KillianLucas
Copy link
Collaborator

Thanks @Arrendy! And thanks @Notnaton for working on this as well! This was totally a mistake I made, luckily committed after the last version that went out to pip, so just a problem for us working off the repo.

Renaming it to run is exactly how it ought to work— if stream=False, we just pull from an identical run with stream=True and return that output. Merging!

@KillianLucas KillianLucas merged commit 9f8af6a into OpenInterpreter:main Jan 18, 2024
0 of 2 checks passed
@MikeBirdTech MikeBirdTech deleted the fix-computer-not-defined branch January 18, 2024 15:25
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.

None yet

3 participants