-
Notifications
You must be signed in to change notification settings - Fork 679
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
chat: flake fixes #1301
chat: flake fixes #1301
Conversation
90f18a4
to
e94a756
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -228,7 +234,7 @@ def handle_function_call(self, run): | |||
# convert to json | |||
stage_str = "convert output to json" | |||
output = json.dumps(output) | |||
except: | |||
except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sensible change, but note that catching Exception
can lead to very annoying behaviours where it catches things that you don't want it to (e.g. the user pressing ctrl-c to kill the program!).
Best to narrow exceptions as much as possible.
@@ -64,20 +68,20 @@ def main(openai_api_key=None, delete_unused=False): | |||
for assistant_file in client.beta.assistants.files.list(assistant_id=assistant.id): | |||
file_assistant_count[assistant_file.id] = file_assistant_count.get(assistant_file.id, 0) + 1 | |||
filename_size_date = get_filename_size_date_from_id(existing_files, assistant_file.id) | |||
print(" id:" + assistant_file.id + " name:" + filename_size_date[0] + " size:" + str(filename_size_date[1]) + " date:" + filename_size_date[2]) | |||
print(" id:" + assistant_file.id + " name:" + filename_size_date[0] + " size:" + str(filename_size_date[1]) + " date:" + filename_size_date[2]) # noqa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f-strings can make your code a little more readable:
print(" id:" + assistant_file.id + " name:" + filename_size_date[0] + " size:" + str(filename_size_date[1]) + " date:" + filename_size_date[2]) # noqa | |
print(f" id:{assistant_file.id} name:{filename_size_date[0]} size:{str(filename_size_date[1])} date:{filename_size_date[2]}) # noqa |
@@ -64,20 +68,20 @@ def main(openai_api_key=None, delete_unused=False): | |||
for assistant_file in client.beta.assistants.files.list(assistant_id=assistant.id): | |||
file_assistant_count[assistant_file.id] = file_assistant_count.get(assistant_file.id, 0) + 1 | |||
filename_size_date = get_filename_size_date_from_id(existing_files, assistant_file.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pull the result apart directly here instead of indexing filename_size_date
:
filename_size_date = get_filename_size_date_from_id(existing_files, assistant_file.id) | |
(file_name, file_size, file_date) = get_filename_size_date_from_id(existing_files, assistant_file.id) |
On the last PR @peterbarker brought up some flake8 fixes so I think this resolves all but one. It doesn't like the long line in chat_openai.py (line 612).
This has been lightly tested in SITL and seems OK.