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

feature(FileManager): adding FileManager to make feasible work with the library in other environment #1573

Merged
merged 6 commits into from
Jan 31, 2025

Conversation

scaliseraoul
Copy link
Contributor

@scaliseraoul scaliseraoul commented Jan 31, 2025


Important

Introduce FileManager abstraction for file operations, replacing direct file system calls across modules, and update tests accordingly.

  • File Management:
    • Introduce FileManager and DefaultFileManager in helpers/filemanager.py for file operations.
    • Replace os file operations with FileManager methods in pandasai/__init__.py, loader.py, and base.py.
  • Functionality:
    • Update create function in pandasai/__init__.py to use FileManager for directory and file operations.
    • Modify DatasetLoader in loader.py to use FileManager for schema file operations.
    • Update DataFrame in base.py to use FileManager for push and pull operations.
  • Removals:
    • Remove FileBasedPrompt class from core/prompts/file_based_prompt.py.
  • Testing:
    • Update tests in test_loader.py, test_sql_loader.py, and test_pandasai_init.py to mock FileManager methods.

This description was created by Ellipsis for bcc04a9. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to bcc04a9 in 1 minute and 29 seconds

More details
  • Looked at 1280 lines of code in 18 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. pandasai/__init__.py:106
  • Draft comment:
    Consider using ConfigManager.get() instead of config.get() for consistency with other parts of the codebase.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new FileManager abstraction to handle file operations, which is a good design choice for flexibility and testing. However, there are some areas where the code can be improved for clarity and consistency.
2. pandasai/data_loader/loader.py:58
  • Draft comment:
    Consider using ConfigManager.get() instead of config.get() for consistency with other parts of the codebase.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new FileManager abstraction to handle file operations, which is a good design choice for flexibility and testing. However, there are some areas where the code can be improved for clarity and consistency.
3. pandasai/dataframe/base.py:167
  • Draft comment:
    Consider using ConfigManager.get() instead of config.get() for consistency with other parts of the codebase.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new FileManager abstraction to handle file operations, which is a good design choice for flexibility and testing. However, there are some areas where the code can be improved for clarity and consistency.
4. pandasai/__init__.py:109
  • Draft comment:
    The error message "Dataset already exists at path: {path}" could be improved for clarity. Consider rephrasing it to provide more context, such as "A dataset with the specified path already exists. Please choose a different path or remove the existing dataset."
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The error message in the create function when a dataset already exists is not very descriptive. It should provide more context to the user.
5. pandasai/data_loader/loader.py:61
  • Draft comment:
    The error message "Schema file not found: {schema_path}" could be improved for clarity. Consider rephrasing it to provide more context, such as "The schema file could not be located at the specified path. Please ensure the path is correct and the file exists."
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The error message in the create_loader_from_path method when the schema file is not found is not very descriptive. It should provide more context to the user.
6. pandasai/data_loader/sql_loader.py:46
  • Draft comment:
    The error message "Failed to execute query for '{source_type}' with: {formatted_query}" could be improved for clarity. Consider rephrasing it to provide more context, such as "The query execution failed for the specified source type. Please check the query syntax and source configuration."
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The error message in the execute_query method when a query fails is not very descriptive. It should provide more context to the user.

Workflow ID: wflow_N6uHl6egM1T7U7xZ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@gventuri gventuri merged commit 0c6738b into sinaptik-ai:main Jan 31, 2025
12 checks 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.

3 participants