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

Add Ecosia Provider, Add OpenaiAccount alias #1854

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Conversation

hlohaus
Copy link
Collaborator

@hlohaus hlohaus commented Apr 18, 2024

Use AsyncClient in API, add web_search parameter in API Improve error messages in Openai

Use AsyncClient in API, add web_search parameter in API
Improve error messages in Openai
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello H Lohaus,

Thank you for your contribution to the project! Here's my review of your pull request titled "Add Ecosia Provider, Add OpenaiAccount alias":

General Feedback

  • Great job on implementing the Ecosia provider and adding the OpenaiAccount alias. It's good to see the use of AsyncClient in the API, which should improve the performance.
  • The addition of the web_search parameter in the API is a thoughtful touch, potentially expanding the functionality.

Code Review

  • In ecosia.py, the implementation of the asynchronous main function looks clean and well-structured. The streaming of completions is a nice feature.
  • The Ecosia class in Ecosia.py is well implemented with clear model aliasing and support for the gpt-3.5-turbo model.
  • The removal of the unnecessary newline in You.py is a good catch.
  • The update to the default_model in OpenRouter.py to use mistralai/mistral-7b-instruct:free is a significant change. Please ensure that this model aligns with the project's requirements.
  • In Openai.py, the addition of the raise_error method improves error handling.
  • The creation of OpenaiAccount.py is a good addition, but please make sure that the authentication requirements are well documented for future contributors.

Suggestions

  • Ensure that all new classes and methods have appropriate docstrings for maintainability.
  • Consider adding unit tests for the new provider to ensure stability and ease future maintenance.
  • Verify that the changes adhere to the project's coding standards and guidelines.

Overall, this is a solid pull request with valuable additions. After addressing the above points, it should be ready for merging.

Best,
g4f Copilot

examples/ecosia.py Show resolved Hide resolved
examples/ecosia.py Show resolved Hide resolved
examples/ecosia.py Show resolved Hide resolved
examples/ecosia.py Show resolved Hide resolved
g4f/Provider/Ecosia.py Show resolved Hide resolved
g4f/gui/client/index.html Show resolved Hide resolved
g4f/gui/client/index.html Show resolved Hide resolved
g4f/gui/client/index.html Show resolved Hide resolved
g4f/gui/client/index.html Show resolved Hide resolved
g4f/gui/client/index.html Show resolved Hide resolved
@hlohaus hlohaus merged commit 718ea7c into xtekky:main Apr 18, 2024
1 check passed
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.

1 participant