Fix #83 Read image data within the convo with the bot #87
Conversation
|
@seratch , how can I add more commits to fix the check errors? |
| ) -> int: | ||
| """Returns the number of tokens used by a list of messages.""" |
There was a problem hiding this comment.
I prefer less changes here. you can extract common parts for supporting new patterns but there is no need to completely rewrite.
There was a problem hiding this comment.
ok, just use ai to help refactor, I think it's too long
| def encode_and_count_tokens(value, encoding): | ||
| if isinstance(value, str): | ||
| return len(encoding.encode(value)) | ||
| elif isinstance(value, list): |
There was a problem hiding this comment.
this pattern is not consistent with calculate_num_tokens
requirements.txt
Outdated
| @@ -4,3 +4,5 @@ openai>=1.30.1,<2 | |||
| tiktoken>=0.6,<0.7 | |||
| # https://github.com/Yelp/elastalert/issues/2306 | |||
| urllib3<2 | |||
| # https://github.com/python-pillow/Pillow | |||
| pillow==0.4.0 | |||
|
please resolve the lint errors on your branch first; you can do so with your fork as these validations do not require any extra settings |
app/openai_ops.py
Outdated
| return None | ||
|
|
||
|
|
||
| def raise_not_supported_error(model: str): |
There was a problem hiding this comment.
this change is not bad but please avoid introducing non-essential changes to your goal. we can do this separately. keep your diff as small as possible
|
@seratch Do you think we should use another PR for the refactor? |
seratch
changed the title
|
@gnuser Thanks for updating the PR! Before doing refactoring, it seems your change still need updates. I will check the behavior and apply a few additional changes. As for refactoring work in general, I am open to any improvements, but it does not mean I always accept them. |
seratch
changed the title
|
Closing this in favor of #88 |
This pull request is indented to resolve #83
using this vision api