-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature/ollama extension #1652
base: main
Are you sure you want to change the base?
Feature/ollama extension #1652
Conversation
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.
❌ Changes requested. Reviewed everything up to b21da05 in 2 minutes and 37 seconds
More details
- Looked at
483
lines of code in7
files - Skipped
2
files when reviewing. - Skipped posting
9
drafted comments based on config settings.
1. extensions/llms/litellm/pandasai_litellm/litellm.py:1
- Draft comment:
Good refactoring of logging imports to avoid duplicates. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. extensions/llms/ollama/README.md:25
- Draft comment:
Ensure installation instructions are clear and commands are correctly formatted. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. extensions/llms/ollama/pandasai_ollama/ollama.py:85
- Draft comment:
Consider checking for the existence of the 'best_of' attribute using hasattr for consistency. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. extensions/llms/ollama/tests/test_ollama.py:112
- Draft comment:
Tests are comprehensive. In future, consider adding tests for error handling in code extraction. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
5. pyproject.toml:31
- Draft comment:
Main project dependencies look up-to-date; ensure that version constraints for pandasai-ollama match released package versions. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. extensions/llms/litellm/pandasai_litellm/litellm.py:1
- Draft comment:
Good improvement moving the logging import to the top. Ensure no duplicate imports remain and maintain a consistent import order. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. extensions/llms/ollama/pandasai_ollama/ollama.py:29
- Draft comment:
When normalizing the base URL (appending '/v1'), consider handling edge cases such as URLs that already have trailing slashes or additional URL components. This ensures robustness if users supply unexpected formats. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The code already handles the main edge case (trailing slashes) using rstrip(). The comment is speculative about "unexpected formats" without providing concrete examples. The current implementation is sufficient for the basic requirement of ensuring "/v1" endpoint. Making it more complex might be premature optimization.
The comment raises a valid point about URL robustness. There could be edge cases like double slashes or malformed URLs that aren't handled.
While URL edge cases exist, the current code handles the basic requirement and common case (trailing slashes). Without concrete examples of problematic URLs, adding more complexity isn't justified.
Delete the comment. It suggests handling hypothetical edge cases without clear examples, and the current implementation already handles the basic requirement adequately.
8. extensions/llms/ollama/README.md:40
- Draft comment:
The README is clear. For improved clarity, consider minor grammatical tweaks such as specifying in the 'Customizable Base URL' section that the URL is normalized to end with '/v1'. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
9. pyproject.toml:28
- Draft comment:
Found a trivial formatting issue: the line defining 'sqlglot = "^25.0.3"' appears to have trailing whitespace at the end. Please remove the extra space to keep the file clean. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_cQlI4EYmqLAS1pGB
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
||
# Ask the question | ||
response = df.chat("Which are the top 5 countries by sales?") | ||
print("Response from Ollama:", response) |
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 integration test currently prints the response. For better automated testing, consider asserting on the output rather than printing it.
print("Response from Ollama:", response) | |
assert response is not None, "Response from Ollama should not be None" |
@@ -0,0 +1,91 @@ | |||
# PandasAI Ollama LLM Extension | |||
|
|||
This extension integrates Ollama language models with PandaAI. It allows you to use Ollama's LLMs as the backend for generating Python code in response to natural language queries on your dataframes. |
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.
There is an inconsistency in the spelling of the project name between the title and the text. The title uses "PandasAI" while the text in line 3 and later sections refers to "PandaAI". For clarity and consistency, please standardize the naming to one consistent form.
This extension integrates Ollama language models with PandaAI. It allows you to use Ollama's LLMs as the backend for generating Python code in response to natural language queries on your dataframes. | |
This extension integrates Ollama language models with PandasAI. It allows you to use Ollama's LLMs as the backend for generating Python code in response to natural language queries on your dataframes. |
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.
Thank you,I will take all that in account, and correct the issues
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.
Pull Request Changes Description:
Overview
- Uses a consistent naming convention ("PandaAI") across the code and documentation.
- Provides clearer installation instructions in the README.
- Supports a customizable base URL (normalized to end with
/v1
) and flexible model parameters (e.g., temperature, max_tokens).- Updates the integration tests to assert expected outputs rather than just printing responses.
Key Changes
Naming Consistency:
- Updated the README and code comments so that the project is consistently referred to as "PandaAI".
Installation Instructions:
- Improved README installation instructions to clearly show how to install and configure the extension.
Configuration & Parameters:
- The OllamaLLM class now uses a dummy API key (since Ollama does not require one) and accepts a custom base URL via the
ollama_base_url
parameter.- Parameters such as model, temperature, and max_tokens are correctly forwarded to the underlying OpenAI client.
Testing Improvements:
- Updated the tests in
extensions/llms/ollama/tests/test_ollama.py
to include proper assertions (e.g., asserting that the response is not None).- Removed the temporary
test_1.py
file that was used for manual testing.Known Issues:
- There is still a known issue where the generated LLM response may not always return the expected Python code snippet (either resulting in a
NoCodeFoundError
or anExecuteSQLQueryNotUsed
error). This is due to the inherent variability in model outputs and will be improved in future updates.How to Test
Installation:
- Clone the repository and navigate to the extension directory.
- Run:
poetry install poetry add pandasai-ollamaConfiguration & Usage:
- Configure via environment variables or directly in code (see updated README).
Run Tests:
- Execute:
poetry run pytest extensions/llms/ollama/testsPlease review the changes and let me know if further modifications are required (my latests changes).
Pull Request Description
Overview
This pull request introduces an extension for PandaAI that integrates with Ollama. The extension (pandasai-ollama) allows users to set a custom base URL and model when using Ollama as their LLM backend. It provides a similar interface to our existing OpenAI integration but uses a dummy API key (since Ollama does not require one) and forwards additional parameters (e.g., model, temperature, max_tokens) to the underlying client.
Key Changes
Custom Base URL Support:
The extension now accepts an
ollama_base_url
parameter. The URL is normalized to always end with/v1
so that users can point to custom deployments of Ollama.Model and Parameter Configuration:
Users can configure the model (e.g.,
"llama3.2:latest"
) and other parameters such astemperature
andmax_tokens
through the extension. The code extracts these parameters and forwards them to the underlying OpenAI client (used as a shim) so that the request is built correctly.Global Configuration Override:
The extension can be set as the active LLM in PandaAI’s global configuration using
pai.config.set(...)
, so that calls todf.chat(...)
will use Ollama.Unit Tests:
Comprehensive tests under
extensions/llms/ollama/tests/test_ollama.py
have been implemented. They verify that:type
property correctly returns"ollama"
.Known Issue
There is a known issue regarding code extraction and validation. Sometimes, the generated response from the model does not include the expected Python code snippet for SQL execution. In such cases:
NoCodeFoundError
when it fails to find any code in the output.execute_sql_query
, the validator raises anExecuteSQLQueryNotUsed
error.This issue appears to be due to the inherent variability in model output. We plan to address this further in subsequent updates. For now, users are advised that if they encounter these errors, the issue is known and relates to how the model formats its response—not to the extension’s functionality per se.
Conclusion
This extension adds support for Ollama as an LLM backend within PandasAI, allowing for a flexible base URL and configurable parameters. While the extension generally works well (as confirmed by our unit tests), the code extraction from model responses may sometimes fail to return the expected Python snippet. We have documented this as a known issue and will continue to improve the robustness of the code extraction process in future updates.
Important
Introduces Ollama extension for PandaAI, allowing custom base URL and model parameters, with comprehensive tests and documentation.
OllamaLLM
class inollama.py
for integrating Ollama LLMs with PandaAI.temperature
,max_tokens
).http://localhost:11434/v1
.OLLAMA_API_KEY
andOLLAMA_BASE_URL
for configuration.README.md
.test_ollama.py
verify base URL setting, parameter configuration, and chat/non-chat modes.test_1.py
demonstrates usage with a sample DataFrame.pandasai-ollama
topyproject.toml
dependencies.README.md
for installation and usage.This description was created by
for b21da05. It will automatically update as commits are pushed.