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

OpenAI and Fireworks vision services #121

Closed
wants to merge 5 commits into from
Closed

Conversation

chadbailey59
Copy link
Contributor

I might try to add Fireworks image gen support tomorrow, but we can also merge this as-is if we need to cut a release

dot-env.template Outdated Show resolved Hide resolved

else:
description = await self.run_vision(frame)
yield TextFrame(description)
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't run_vision be async already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, but naming things is hard 😂 the way you implemented the run_vision method for moondream returns a string that you wrap in a text frame, but the the way the OpenAI module returns completions is an AsyncGenerator, so I needed another run_vision method. It's clunky but I'm out of ideas

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should make Moondream return AsyncGenerator :-). I'll fix that tomorrow.

@chadbailey59 chadbailey59 requested a review from aconchillo April 12, 2024 18:53
@vipyne
Copy link
Member

vipyne commented Oct 9, 2024

ok to close @chadbailey59 ?

@vipyne
Copy link
Member

vipyne commented Oct 22, 2024

closing because this is stale

@vipyne vipyne closed this Oct 22, 2024
@aconchillo aconchillo deleted the cb/fireworks-vision branch October 23, 2024 20:57
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.

3 participants