fix(test_cases): test cases for pandas 3.0 #1506
Conversation
Add 's' in the end of keyword "clarification_question"
agent.clarification_question('What is the GDP of the United States?')
* Add /app to PYTHONPATH * fix for issue #1415 --------- Co-authored-by: Giuseppe Coco <[email protected]>
Co-authored-by: Giuseppe Coco <[email protected]>
Add documentation of how to use pandasai.json file
#1432) * fix[output_format]: accept dataframe dict as output and secure sql query execution * fix: ruff errors
* chore[Security]: restrict libs to allow specific functionalities * remove: extra lib handling * fix: ruff errors * fix: error message for bad import * fix: add io library in the blacklist
In SmartDataframe class the last_code_generated property returned _agent.last_code_executed instead of _agent.last_code_generated. In SmartDatalake it is implemented properly. Co-authored-by: Bence Kecskés <[email protected]>
-Simplified the type of children to just React.ReactNode, which is the standard type for React components' children prop.
* fix: make seaborn as an optional dependency * fix: linting errors
* feat(security): add security config to disable it * fix: linting errors * fix(safety): push exact match for get attributes * add additional test case * fix: test case * fix: linting errors * fix: linting errors * docs(config): update config doc to add new config attribute
dosubot
bot
added
the
size:XL
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 0be115f in 2 minutes and 40 seconds
More details
- Looked at
7021lines of code in104files - Skipped
1files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. tests/unit_tests/core/prompts/test_prompts.py:38
- Draft comment:
Consider adding more detailed assertions to verify the prompt's structure and content beyond just checking for the presence of the visualization library. - Reason this comment was not posted:
Confidence changes required:50%
The test for theget_chat_promptfunction checks if the prompt contains the expected visualization library. However, it doesn't verify if the prompt is correctly formatted or if it includes all necessary components. Adding more detailed assertions could improve the test's robustness.
2. tests/unit_tests/core/prompts/test_prompts.py:58
- Draft comment:
Consider adding more detailed assertions to verify the prompt's structure and content beyond just checking for the presence of the visualization library. - Reason this comment was not posted:
Confidence changes required:50%
The test forget_chat_prompt_for_sqlchecks if the prompt contains the expected visualization library. However, it doesn't verify if the prompt is correctly formatted or if it includes all necessary components. Adding more detailed assertions could improve the test's robustness.
3. tests/unit_tests/core/prompts/test_prompts.py:67
- Draft comment:
Consider adding assertions to verify the content of the prompt, ensuring it includes the expected code and error message. - Reason this comment was not posted:
Confidence changes required:50%
The test forget_correct_error_promptonly checks the type of the returned prompt. It would be beneficial to also verify the content of the prompt to ensure it includes the expected code and error message.
4. tests/unit_tests/core/prompts/test_prompts.py:77
- Draft comment:
Consider adding assertions to verify the content of the prompt, ensuring it includes the expected SQL code and error message. - Reason this comment was not posted:
Confidence changes required:50%
The test forget_correct_error_prompt_for_sqlonly checks the type of the returned prompt. It would be beneficial to also verify the content of the prompt to ensure it includes the expected SQL code and error message.
5. tests/unit_tests/core/prompts/test_prompts.py:87
- Draft comment:
Consider adding assertions to verify the content of the prompt, ensuring it includes the expected code and output type. - Reason this comment was not posted:
Confidence changes required:50%
The test forget_correct_output_type_error_promptonly checks the type of the returned prompt. It would be beneficial to also verify the content of the prompt to ensure it includes the expected code and output type.
6. tests/unit_tests/prompts/test_sql_prompt.py:9
- Draft comment:
The docstring should be updated to reflect the correct class being tested. It should mention 'generate python code with SQL prompt class' instead of 'correct error prompt class'. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_jdgJNcYXafE3C217
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 58fcff7 in 1 minute and 10 seconds
More details
- Looked at
58lines of code in1files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. .github/workflows/ci.yml:44
- Draft comment:
Removing--with devfrompoetry installmay lead to missing development dependencies required for testing and CI tasks. Consider adding it back to ensure all necessary dependencies are installed. This applies to all similar changes in this file. - Reason this comment was not posted:
Comment did not seem useful.
2. .github/workflows/ci.yml:45
- Draft comment:
Consider rephrasing the echo message for clarity:
echo "Installing dependencies for directory: $dir"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
3. .github/workflows/ci.yml:55
- Draft comment:
Consider rephrasing the echo message for clarity:
echo "Installing dependencies for directory: $dir"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
4. .github/workflows/ci.yml:65
- Draft comment:
Consider rephrasing the echo message for clarity:
echo "Installing dependencies for directory: $dir"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
5. .github/workflows/ci.yml:78
- Draft comment:
Consider rephrasing the Write-Host message for clarity:
Write-Host "Installing dependencies for directory: $_"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
6. .github/workflows/ci.yml:89
- Draft comment:
Consider rephrasing the Write-Host message for clarity:
Write-Host "Installing dependencies for directory: $_"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
7. .github/workflows/ci.yml:100
- Draft comment:
Consider rephrasing the Write-Host message for clarity:
Write-Host "Installing dependencies for directory: $_"
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is clear and concise, but the grammar can be improved for better readability.
Workflow ID: wflow_7FrP2Nk7OEH6tKNK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -8,19 +8,19 @@ | |||
| [](https://pepy.tech/project/pandasai) [](https://opensource.org/licenses/MIT) | |||
| [](https://colab.research.google.com/drive/1ZnO-njhL7TBOYPZaqvMvGtsjckZKrv2E?usp=sharing) | |||
|
|
|||
| PandasAI is a Python platform that makes it easy to ask questions to your data in natural language. It helps non-technical users to interact with their data in a more natural way, and it helps technical users to save time and effort when working with data. | |||
| PandasAI is a Python platform that makes it easy to ask questions to your data in natural language. It helps non-technical users to interact with their data in a more natural way, and it helps technical users to save time, and effort when working with data. | |||
There was a problem hiding this comment.
@ArslanSaleem this new punctuation seems to be incorrect
There was a problem hiding this comment.
Yes looking at the test extension deps not getting installed in github flow
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 0958749 in 2 minutes and 55 seconds
More details
- Looked at
30lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. .github/workflows/ci.yml:68
- Draft comment:
Usingsudoin CI scripts can lead to potential security issues. Consider alternative approaches to manage permissions withoutsudo. This applies to lines 93 and 118 as well. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While using sudo in CI can sometimes be a security concern, in this case it's being used to fix permission issues with Poetry virtualenvs on macOS runners. The sudo command is limited to a specific directory path and user. The CI environment is ephemeral and isolated. Alternative approaches would likely be more complex and potentially less reliable.
The comment raises a valid general security principle. There could be alternative ways to handle permissions that don't require sudo.
The sudo usage here is narrow in scope, only affecting Poetry virtualenv directories in an isolated CI environment. The security risk is minimal and the solution is pragmatic.
Delete the comment. While the security concern is valid in principle, the sudo usage here is reasonable and necessary for the CI workflow to function properly. Alternative approaches would add unnecessary complexity.
2. .github/workflows/ci.yml:65
- Draft comment:
The message "Cleaning up dependencies for $dir" can be improved for clarity. Consider changing it to:
Cleaning up dependencies in $dir
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is not grammatically correct. It should be improved for clarity.
Workflow ID: wflow_7vtXxuAOOzAqg7Jn
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bcec071 in 1 minute and 32 seconds
More details
- Looked at
43lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/ci.yml:62
- Draft comment:
Removing the dependency cleanup step might lead to increased disk usage and potential conflicts if dependencies are not properly isolated. Ensure that there are other mechanisms in place to handle dependency isolation and cleanup. - Reason this comment was not posted:
Confidence changes required:50%
The removal of the dependency cleanup step might lead to increased disk usage and potential conflicts if dependencies are not properly isolated. However, if the cleanup is not necessary due to other mechanisms in place, this change could be justified.
Workflow ID: wflow_1wad6Fj3I9Tz1oyE
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2da1398 in 1 minute and 53 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. extensions/llms/bedrock/tests/test_bedrock_claude.py:71
- Draft comment:
The removal of themockerparameter is correct as it is not used in the test. - Reason this comment was not posted:
Confidence changes required:0%
Themockerparameter was removed from thetest_callmethod, but it is not used in the method body. This change is correct and does not affect the test functionality.
2. extensions/llms/bedrock/tests/test_bedrock_claude.py:71
- Draft comment:
Remove the unusedmockerparameter from thetest_callfunction.
def test_call(self, prompt):
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_10EfWGOx1MdAOqCz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7570c2b in 1 minute and 28 seconds
More details
- Looked at
28lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/ci.yml:55
- Draft comment:
The removal ofpoetry add [email protected]is appropriate. Dependencies should be managed throughpyproject.tomlto ensure consistency. - Reason this comment was not posted:
Confidence changes required:10%
The removal ofpoetry add [email protected]from the CI workflow is correct becausepytestshould be managed through thepyproject.tomlfile, ensuring consistent dependency management across environments.
Workflow ID: wflow_MlQ8l2yrFslSYdIO
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on f2f15ce in 2 minutes and 19 seconds
More details
- Looked at
34lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/ci.yml:55
- Draft comment:
The message "Installing dependencies for $dir" can be improved for clarity. Consider changing it to:
"Installing dependencies in $dir"
This applies to lines 76 and 97 as well.
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the script is not grammatically correct. It should be improved for clarity.
Workflow ID: wflow_LvvFzGP9haH2ikca
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.
.github/workflows/ci.yml
Outdated
| ( | ||
| cd "$dir" || exit | ||
| poetry install --all-extras | ||
| poetry add [email protected] |
There was a problem hiding this comment.
Consider adding pytest dependencies globally once instead of repeating poetry add [email protected], poetry add [email protected], and poetry add [email protected] in each loop. This will reduce redundancy and improve maintainability.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on f8e5f23 in 2 minutes and 3 seconds
More details
- Looked at
28lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_HdmBefEneimRC4Ta
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.
| @@ -116,7 +118,10 @@ def test_get_cache_file_path_with_destination_path(self, sample_schema): | |||
| loader.schema = sample_schema | |||
| loader.dataset_path = "test/users" | |||
| cache_path = loader._get_cache_file_path() | |||
| assert cache_path == "datasets/test/users/users.parquet" | |||
| if sys.platform.startswith("win"): | |||
There was a problem hiding this comment.
sys is not imported. Add import sys at the top of the file.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 59b42ed in 1 minute and 46 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. tests/unit_tests/dataframe/test_loader.py:6
- Draft comment:
The import ofsysis unnecessary and can be removed to clean up the code. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is incorrect because thesysmodule is actively used in the code for platform-specific path handling. The import is necessary and removing it would break the tests. This is a clear case where the automated tool made a mistake.
Could there be a better way to handle path separators that doesn't require platform checks? Maybe the tool was trying to suggest using pathlib instead?
While there might be better ways to handle paths, that's a separate issue. The current code relies on sys.platform and the import is necessary for the code to function.
The comment should be deleted because it is incorrect - the sys import is necessary for the current implementation.
2. tests/unit_tests/dataframe/test_loader.py:6
- Draft comment:
Consider ordering the import statements alphabetically for better readability. - Reason this comment was not posted:
Confidence changes required:50%
The import statements are not in alphabetical order, which is a common convention for readability.
Workflow ID: wflow_uGRgXPvpgrHGXLPo
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7e207eb in 2 minutes and 48 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. tests/unit_tests/dataframe/test_loader.py:1
- Draft comment:
Duplicate import of 'sys'. Remove the redundant import statement. - Reason this comment was not posted:
Confidence changes required:50%
The import statement for 'sys' is duplicated, which is unnecessary and should be removed.
Workflow ID: wflow_E4AHGfgD6bpLWQJ7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3982c45 in 1 minute and 28 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. tests/unit_tests/dataframe/test_loader.py:123
- Draft comment:
Consider usingos.path.joinfor constructing file paths to ensure cross-platform compatibility. This comment applies to line 135 as well. - Reason this comment was not posted:
Confidence changes required:50%
The code uses hardcoded path separators which can cause issues on different operating systems. Usingos.path.joinis a better practice for cross-platform compatibility.
Workflow ID: wflow_g8KlMALU7C7AUUYs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on ecafc64 in 2 minutes and 45 seconds
More details
- Looked at
47lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/ci.yml:121
- Draft comment:
The repeated addition of the same dependencies in multiple places can be optimized by defining them once. However, this is not a grammar issue, so no comment is needed. - Reason this comment was not posted:
Confidence changes required:0%
The repeated addition of the same dependencies in multiple places can be optimized by defining them once. However, this is not a grammar issue, so no comment is needed.
Workflow ID: wflow_JO3zUupZ0RYmTzjq
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.
.github/workflows/ci.yml
Outdated
| Push-Location $_.FullName | ||
| poetry install --all-extras | ||
| poetry add [email protected] |
There was a problem hiding this comment.
The repeated addition of pytest, pytest-mock, and pytest-cov in multiple places is inefficient. Consider adding these dependencies to the pyproject.toml file or a common setup script to avoid redundancy and potential version mismatches.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 64e51e3 in 1 minute and 20 seconds
More details
- Looked at
106lines of code in3files - Skipped
1files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. extensions/ee/connectors/bigquery/pandasai_bigquery/__init__.py:15
- Draft comment:
TheGoogleBigQueryConnectorwas removed from imports but is still listed in__all__. This will cause an ImportError if accessed. Ensure__all__only includes available objects. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. extensions/ee/connectors/bigquery/tests/test_bigquery.py:41
- Draft comment:
Ensure that patchingpandas.DataFramedoes not interfere with other tests or parts of the code. Consider using a more specific patch target if necessary. - Reason this comment was not posted:
Confidence changes required:50%
The test forload_from_bigqueryis well-structured, but it is important to ensure that thepandas.DataFramepatching is correctly applied to avoid any unexpected behavior.
3. extensions/ee/connectors/bigquery/tests/test_bigquery.py:17
- Draft comment:
Comment is not concise. Suggestion:
# Mock query result with sample data
- Reason this comment was not posted:
Confidence changes required:50%
The comment on line 17 is not grammatically correct. It should be more concise and clear.
Workflow ID: wflow_sNXGkYrpEnfZQDwm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4d84252 in 1 minute and 5 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. extensions/ee/connectors/bigquery/pyproject.toml:12
- Draft comment:
Ensure thatsqlalchemy-bigqueryis necessary for the BigQuery integration and that version^1.8.0is compatible with other dependencies. - Reason this comment was not posted:
Confidence changes required:50%
The addition ofsqlalchemy-bigqueryas a dependency seems appropriate given the context of BigQuery integration. However, it's important to ensure that this addition is necessary and that the version specified is compatible with the rest of the dependencies.
Workflow ID: wflow_m2BScNjkWCa0tJVO
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ea9a37d in 2 minutes and 6 seconds
More details
- Looked at
77lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. .github/workflows/ci.yml:136
- Draft comment:
Missing test execution step after installing dependencies. Addpoetry run pytest tests/to ensure tests are executed. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. .github/workflows/ci.yml:112
- Draft comment:
The commandecho "C:\Users\runneradmin\AppData\Roaming\Python\Scripts" >> $env:GITHUB_PATHshould useWrite-Outputinstead ofechofor better compatibility in PowerShell.
Write-Output "C:\Users\runneradmin\AppData\Roaming\Python\Scripts" >> $env:GITHUB_PATH
- Reason this comment was not posted:
Confidence changes required:50%
The comment should address the grammar issue in the echo command for Windows.
Workflow ID: wflow_6EwwhqlPCNoRtLak
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
Skipped PR review on 7c4119b because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
gventuri
changed the title
Important
Add comprehensive unit tests for
pandasaicomponents, including code execution, environment setup, code generation, security checks, and response parsing.TestCodeExecutorintest_code_execution.pyforCodeExecutorexecution and environment handling.TestEnvironmentFunctionsintest_environment.pyfor environment setup and dependency management.TestCodeCleanerintest_code_cleaning.pyforCodeCleanerimport and DataFrame handling.TestCodeSecurityCheckerintest_code_security.pyfor detecting malicious code.TestCodeRequirementValidatorintest_code_validation.pyfor SQL execution validation.TestChatPromptsintest_prompts.pyfor prompt generation and error handling.TestDataFrameintest_dataframe.pyfor DataFrame operations and agent interactions.TestDatasetLoaderintest_loader.pyfor data loading and caching.TestResponseParserintest_responses.pyfor response parsing.TestBambooLLMintest_bamboo_llm.pyfor API interactions.TestCorrectErrorPromptintest_correct_error_prompt.pyfor error prompt handling.TestGeneratePythonCodePromptintest_generate_python_code_prompt.pyfor code generation prompts.TestGeneratePythonCodeWithSQLPromptintest_sql_prompt.pyfor SQL code generation prompts.TestPandasAIInitintest_pandasai_init.pyfor initialization and dataset loading.This description was created by
for ea9a37d. It will automatically update as commits are pushed.