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 FlowGpt provider, Fix issue with None values in api #1625

Merged
merged 4 commits into from
Feb 25, 2024

Conversation

hlohaus
Copy link
Collaborator

@hlohaus hlohaus commented Feb 24, 2024

No description provided.

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.

Thank you, H Lohaus, for your contribution to the project.

Review of Pull Request: Add FlowGpt provider, Fix issue with None values in api

Summary

This pull request introduces a new provider, FlowGpt, and addresses an issue where None values were being incorrectly passed in the API. The changes are well-structured and align with the project's coding standards.

Detailed Comments

Addition of FlowGpt Provider

  • File: g4f/Provider/FlowGpt.py
  • The implementation of the FlowGpt provider is comprehensive, covering all necessary configurations and model support. It's great to see support for a wide range of models including GPT-4, GPT-3.5 variants, Google Gemini, Claude-v2, and Llama2-13b. The use of async/await for asynchronous network calls is appropriate, ensuring non-blocking I/O operations.

Update to __init__.py in Provider and api Directories

  • Files: g4f/Provider/__init__.py and g4f/api/__init__.py
  • The inclusion of FlowGpt in the provider's init file is correctly done, ensuring it is recognized and utilized by the system.
  • The modification in api/__init__.py to use config.dict(exclude_none=True) is a smart fix for the issue with None values. This ensures that any None values are excluded from the dictionary passed to the function, preventing errors related to unexpected None values.

Minor Code Style Adjustments

  • File: g4f/gui/client/js/chat.v1.js
  • The adjustment to prompt_lock variable declaration improves code readability and consistency with the surrounding code style.

Recommendations

  • Testing: Ensure comprehensive testing, especially around the new provider integration and the API fix. This includes testing with different models, handling various message histories, and verifying the exclusion of None values does not negatively impact other functionalities.
  • Documentation: It would be beneficial to update the project's documentation to include information about the newly supported FlowGpt provider and any necessary configurations or limitations.

Conclusion

The pull request is well-prepared and introduces valuable enhancements to the project. After addressing the recommendations above and ensuring thorough testing, it should be in good shape for merging.

Keep up the great work!

"nsfw": False,
"question": messages[-1]["content"],
"history": [{"role": "assistant", "content": "Hello, how can I help you today?"}, *messages[:-1]],
"system": "You are helpful assitant. Follow the user's instructions carefully. Respond using markdown",

Choose a reason for hiding this comment

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

The system field contains a typo: assitant should be assistant. Also, ensure the system description is accurate and consider externalizing such strings for easier management and updates.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@hlohaus hlohaus merged commit db58b58 into xtekky:main Feb 25, 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.

None yet

1 participant