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.

Sign up for GitHub

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

Refactored by Sourcery AI #188

Merged
merged 2 commits into from
Jun 19, 2023
Merged

Refactored by Sourcery AI #188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff3495b
'Refactored by Sourcery'
Jun 19, 2023
75fee4e
Refactored by Sourcery AI
LopeKinz Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions gpt_engineer/ai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def fassistant(self, msg):

def next(self, messages: list[dict[str, str]], prompt=None):
if prompt:
messages = messages + [{"role": "user", "content": prompt}]
messages += [{"role": "user", "content": prompt}]
Copy link
Owner

Choose a reason for hiding this comment

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

Extending a list modifies the argument in python no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe both lines do exactly the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes they do.

Copy link

@tmpolaczyk tmpolaczyk Jun 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

They don't, += modifies the variable for the caller, the old code creates a new variable. I was testing this out while you merged lol:

def foo1(messages):
    messages += ["role"]

def foo2(messages):
    messages = messages + ["role"]

m1 = ["m1"]
foo1(m1)
print(m1)
m2 = ["m2"]
foo2(m2)
print(m2)

This prints

['m1', 'role']
['m2']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what python version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://python-reference.readthedocs.io/en/latest/docs/operators/addition_assignment.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

Call them the other way around:

def foo1(messages):
    messages += ["role"]

def foo2(messages):
    messages = messages + ["role"]

m2 = ["m2"]
foo2(m2)
print(m2)

m1 = ["m1"]
foo1(m1)
print(m1)

this prints:

['m2']
['m1', 'role']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just opened a pr without +=

Copy link

Choose a reason for hiding this comment

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

Oops, copied the wrong output, yeah it was:

['m2']
['m1', 'role']

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to clarify

in foo1 we are modifying the messages variable given as parameter

in foo2 we are creating a new list and the original messages remains unchanged

In the actual code we are returning messages so it would be like adding a return in foo2 if you want to compare.


logger.debug(f"Creating a new chat completion: {messages}")
response = openai.ChatCompletion.create(
Expand All @@ -56,6 +56,6 @@ def next(self, messages: list[dict[str, str]], prompt=None):
print(msg, end="")
chat.append(msg)
print()
messages = messages + [{"role": "assistant", "content": "".join(chat)}]
messages += [{"role": "assistant", "content": "".join(chat)}]
logger.debug(f"Chat completion finished: {messages}")
return messages
7 changes: 3 additions & 4 deletions gpt_engineer/db.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ def __init__(self, path):
def __getitem__(self, key):
full_path = self.path / key

if full_path.is_file():
with full_path.open("r", encoding="utf-8") as f:
return f.read()
else:
if not full_path.is_file():
raise KeyError(key)
with full_path.open("r", encoding="utf-8") as f:
return f.read()

def __setitem__(self, key, val):
full_path = self.path / key
Expand Down
4 changes: 2 additions & 2 deletions gpt_engineer/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def main(
logging.basicConfig(level=logging.DEBUG if verbose else logging.INFO)

input_path = Path(project_path).absolute()
memory_path = input_path / (run_prefix + "memory")
workspace_path = input_path / (run_prefix + "workspace")
memory_path = input_path / f"{run_prefix}memory"
workspace_path = input_path / f"{run_prefix}workspace"

if delete_existing:
# Delete files and subdirectories in paths
Expand Down
4 changes: 2 additions & 2 deletions scripts/benchmark.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def main(
benchmarks = []
for bench_folder in folders:
if os.path.isdir(bench_folder):
print("Running benchmark for {}".format(bench_folder))
print(f"Running benchmark for {bench_folder}")

log_path = bench_folder / "log.txt"
log_file = open(log_path, "w")
Expand All @@ -44,7 +44,7 @@ def main(
benchmarks.append((bench_folder, process, log_file))

print("You can stream the log file by running:")
print("tail -f {}".format(log_path))
print(f"tail -f {log_path}")
print()

for bench_folder, process, file in benchmarks:
Expand Down