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.

Sign up for GitHub

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

fix(prompt_path): path issue if pandasai used outside of pandas env #909

Merged
merged 1 commit into from
Jan 30, 2024
Merged

fix(prompt_path): path issue if pandasai used outside of pandas env #909

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pandasai/prompts/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import re
from jinja2 import Environment, FileSystemLoader
from typing import Optional
import os
from pathlib import Path


class BasePrompt:
Expand All @@ -20,7 +22,10 @@ def __init__(self, **kwargs):
env = Environment()
self.prompt = env.from_string(self.template)
elif self.template_path:
env = Environment(loader=FileSystemLoader("pandasai/prompts/templates"))
# find path to template file
current_dir_path = Path(__file__).parent
path_to_template = os.path.join(current_dir_path, "templates")
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (llm): Using os.path.join with pathlib.Path objects is mixing path handling libraries. Consider using Path methods for consistency and clarity.

env = Environment(loader=FileSystemLoader(path_to_template))
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (llm): Ensure that the path_to_template is a string when passed to FileSystemLoader, as pathlib.Path objects are not implicitly converted to strings in all environments.

Comment on lines +26 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic to dynamically locate the template file path using os and pathlib is correctly implemented. However, consider using pathlib exclusively for path operations to maintain consistency and leverage its more modern and object-oriented approach over os.path.

- path_to_template = os.path.join(current_dir_path, "templates")
+ path_to_template = current_dir_path / "templates"
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
current_dir_path = Path(__file__).parent
path_to_template = os.path.join(current_dir_path, "templates")
env = Environment(loader=FileSystemLoader(path_to_template))
current_dir_path = Path(__file__).parent
path_to_template = current_dir_path / "templates"
env = Environment(loader=FileSystemLoader(path_to_template))

The approach to finding the template path and loading it is sound, but it's important to handle potential exceptions that could arise from file not found or access errors when attempting to load the template. Consider adding error handling around the template loading logic to provide more informative error messages to the user or to log such incidents for debugging purposes.

try:
    env = Environment(loader=FileSystemLoader(path_to_template))
    self.prompt = env.get_template(self.template_path)
except TemplateNotFound as e:
    # Handle or log the error appropriately
    raise CustomException(f"Template not found: {self.template_path}") from e
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
current_dir_path = Path(__file__).parent
path_to_template = os.path.join(current_dir_path, "templates")
env = Environment(loader=FileSystemLoader(path_to_template))
current_dir_path = Path(__file__).parent
path_to_template = os.path.join(current_dir_path, "templates")
try:
env = Environment(loader=FileSystemLoader(path_to_template))
self.prompt = env.get_template(self.template_path)
except TemplateNotFound as e:
# Handle or log the error appropriately
raise CustomException(f"Template not found: {self.template_path}") from e

self.prompt = env.get_template(self.template_path)

def render(self):
Expand Down
Loading