refactor: move prompt templates to files (#593) #596
Conversation
* (refactor): define base prompt class as an abstract * (refactor): attribute `text` rename to `template` (since it's essentially is a template) * (feat): add FileBasedPrompt
* (style): resolve naming issues * (tests): CRLF to LF on win platforms
* (fix): revert undesirable change (during autorefactoring the name of prompt's attribute was excessively renamed where it shouldn't have been)
* (docs): update docstrings, update custom-prompts.md * (chore): enhance type hinting * (fix): add missing `FileBasedPrompt` in module __init__.py
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #596 +/- ##
==========================================
- Coverage 83.90% 83.65% -0.25%
==========================================
Files 55 55
Lines 2690 2717 +27
==========================================
+ Hits 2257 2273 +16
- Misses 433 444 +11
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
WalkthroughThis pull request introduces a significant refactoring of the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (26)
- assets/prompt-templates/correct_error_prompt.tmpl (1 hunks)
- assets/prompt-templates/generate_python_code.tmpl (1 hunks)
- docs/custom-prompts.md (2 hunks)
- pandasai/init.py (2 hunks)
- pandasai/llm/azure_openai.py (2 hunks)
- pandasai/llm/base.py (5 hunks)
- pandasai/llm/fake.py (2 hunks)
- pandasai/llm/huggingface_text_gen.py (5 hunks)
- pandasai/llm/langchain.py (2 hunks)
- pandasai/llm/openai.py (2 hunks)
- pandasai/prompts/init.py (1 hunks)
- pandasai/prompts/base.py (2 hunks)
- pandasai/prompts/correct_error_prompt.py (1 hunks)
- pandasai/prompts/generate_python_code.py (1 hunks)
- pandasai/smart_datalake/init.py (2 hunks)
- tests/connectors/test_base.py (1 hunks)
- tests/llms/test_base_hf.py (3 hunks)
- tests/llms/test_google_palm.py (2 hunks)
- tests/llms/test_huggingface_text_gen.py (4 hunks)
- tests/llms/test_langchain_llm.py (2 hunks)
- tests/llms/test_openai.py (2 hunks)
- tests/prompts/test_base_prompt.py (1 hunks)
- tests/prompts/test_correct_error_prompt.py (2 hunks)
- tests/prompts/test_generate_python_code_prompt.py (3 hunks)
- tests/test_smartdataframe.py (4 hunks)
- tests/test_smartdatalake.py (2 hunks)
Files skipped from review due to trivial changes (12)
- assets/prompt-templates/correct_error_prompt.tmpl
- pandasai/llm/fake.py
- pandasai/prompts/init.py
- pandasai/prompts/correct_error_prompt.py
- tests/connectors/test_base.py
- tests/llms/test_huggingface_text_gen.py
- tests/llms/test_langchain_llm.py
- tests/llms/test_openai.py
- tests/prompts/test_base_prompt.py
- tests/prompts/test_correct_error_prompt.py
- tests/prompts/test_generate_python_code_prompt.py
- tests/test_smartdatalake.py
Additional comments (Suppressed): 39
assets/prompt-templates/generate_python_code.tmpl (1)
- 1-30: The new hunk is a template file for generating Python code prompts. It provides placeholders for dataframes, conversation, default imports, and engine dataframe name. The template also includes instructions on how to update the initial Python code based on the last question in the conversation. This change enhances modularity by separating the prompt content from the logic of the
GeneratePythonCodePromptclass.pandasai/llm/langchain.py (2)
1-4: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that theAbstractPromptclass provides all the necessary functionality previously provided byPrompt.16-18: The function signature for
callhas been updated to useAbstractPromptinstead ofPrompt. Make sure that all calls to this function throughout the codebase have been updated to match the new signature.pandasai/llm/openai.py (2)
18-18: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.88-88: The function signature for
callhas been updated to accept anAbstractPromptinstead of aPrompt. Make sure that all calls to this function throughout the codebase have been updated to match the new signature.pandasai/llm/azure_openai.py (2)
21-21: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin this file have been updated accordingly.108-108: The function signature for
call()has been updated to accept anAbstractPromptobject instead of aPromptobject. Make sure that all calls to this function throughout the codebase pass an instance of a class that extendsAbstractPrompt.pandasai/__init__.py (2)
43-43: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.115-115: The type hint for
non_default_promptshas been updated fromPrompttoAbstractPrompt. This change is consistent with the renaming of the base class. Make sure that any custom prompts passed to this function are subclasses ofAbstractPrompt.pandasai/prompts/generate_python_code.py (3)
36-37: The base class
Prompthas been renamed toFileBasedPrompt. This change is consistent with the PR summary and seems to be a part of the refactoring effort to improve modularity. Ensure that all instances ofPrompthave been updated toFileBasedPromptthroughout the codebase.42-43: The
_path_to_templateattribute points to the new template file for this prompt. Make sure that the specified file exists at the given path and contains the expected content.44-50: The
__init__()method now sets two variables,default_importandengine_df_name, using theset_var()method before callingsuper().__init__(**kwargs). This is a change from the old hunk where these variables were set after the super call. This change should not cause any issues as long asset_var()does not rely on any attributes or methods that are initialized in the superclass's__init__()method.tests/llms/test_google_palm.py (2)
6-12: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.19-25: The
MockPromptclass used in thepromptfixture has been renamed toMockAbstractPrompt, and itstextattribute has been renamed totemplate. This change aligns with the renaming ofPrompttoAbstractPromptand the replacement of thetextattribute withtemplatein the base class. Ensure that all instances ofMockPromptand itstextattribute have been updated throughout the test suite.docs/custom-prompts.md (3)
16-34: The new hunk demonstrates how to create a custom prompt by subclassing
AbstractPromptand overriding thetemplateproperty. This is a good example of using inheritance and encapsulation, two principles of object-oriented programming. The code looks correct and should work as expected if themy_custom_valueargument is provided when creating an instance ofMyCustomPrompt.44-59: This hunk shows how to use
FileBasedPromptto create a custom prompt with the template content stored in a file. It's important to ensure that the path specified in_path_to_templateis correct and the file exists at that location. If the file doesn't exist or can't be read, it could cause runtime errors.66-80: This hunk updates the previous example of creating a custom prompt to use the new
AbstractPromptclass and thetemplateattribute instead oftext. The change is consistent with the rest of the PR and should not cause any issues as long as thedfsandconversationvariables are available in the context where the template is rendered.tests/test_smartdataframe.py (4)
20-20: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.380-384: The custom prompt class
CustomPromptnow extendsAbstractPromptinstead ofPrompt, and usestemplateinstead oftext. This change aligns with the refactoring of the prompt system. However, ensure that the template string is correctly formatted and all variables within{}are available in the context when this prompt is used.401-407: The
ReplacementPromptclass now extendsAbstractPromptand overrides thetemplateproperty instead of defining atextattribute. This change is consistent with the refactoring of the prompt system. However, ensure that the return value of thetemplateproperty is correctly formatted and all variables within{}are available in the context when this prompt is used.515-517: The
custom_promptsdictionary now uses an instance ofGeneratePythonCodePromptinstead ofPrompt. This change is consistent with the introduction of the newFileBasedPromptsubclass and the refactoring of the prompt system. Ensure that theGeneratePythonCodePromptclass is correctly implemented and its template file contains valid content.pandasai/smart_datalake/__init__.py (2)
35-37: The import statement for
Prompthas been replaced withAbstractPrompt. This change aligns with the refactoring of the prompt system wherePromptis renamed toAbstractPrompt. Ensure that all references toPromptin the codebase have been updated toAbstractPrompt.204-223: The function
_get_prompt()now returns an instance ofAbstractPromptinstead ofPrompt. The argumentdefault_promptalso expects a type ofAbstractPromptinstead ofPrompt. This change is consistent with the renaming ofPrompttoAbstractPrompt. Make sure that all calls to this function are updated accordingly.tests/llms/test_base_hf.py (3)
7-7: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.19-22: The fixture
promptnow uses a mock classMockAbstractPromptthat extendsAbstractPromptinstead ofPrompt. The attributetexthas been replaced withtemplate, reflecting its role as a template for generating prompts. This change aligns with the refactoring of the prompt system in thepandas-aiproject.65-68: Similar to the previous comment, the test
test_call_removes_original_promptnow uses a mock classMockAbstractPromptthat extendsAbstractPromptinstead ofPrompt. The attributetexthas been replaced withtemplate.pandasai/llm/base.py (5)
32-32: The import statement has been updated to reflect the renaming of
PrompttoAbstractPrompt. Ensure that all references toPromptin the codebase have been updated accordingly.123-123: The function signature for
call()has been updated to accept anAbstractPromptinstead of aPrompt. Make sure all calls to this function throughout the codebase have been updated to match the new signature.137-137: The function signature for
generate_code()has been updated to accept anAbstractPromptinstead of aPrompt. Make sure all calls to this function throughout the codebase have been updated to match the new signature.337-337: The function signature for
call()in theHuggingFaceLLMclass has been updated to accept anAbstractPromptinstead of aPrompt. Make sure all calls to this function throughout the codebase have been updated to match the new signature.432-432: The function signature for
call()in theBaseGoogleclass has been updated to accept anAbstractPromptinstead of aPrompt. Make sure all calls to this function throughout the codebase have been updated to match the new signature.pandasai/llm/huggingface_text_gen.py (5)
2-8: The import statement for
AbstractPromptis correct and aligns with the changes described in the PR summary. Ensure that all references toPrompthave been updated toAbstractPromptthroughout the codebase.14-20: No significant changes were made in this hunk, only a minor change in the formatting of the
temperaturevalue from1E-3to1e-3. Both are equivalent in Python.29-35: No significant changes were made in this hunk, only a minor change in the formatting of the
forloop. The parentheses around(key, val)in the old hunk are not necessary in Python and have been correctly removed in the new hunk.63-71: The function signature of
call()has been updated to accept anAbstractPromptinstead of aPrompt. This aligns with the changes described in the PR summary. Also, the attributetexthas been replaced withtemplatein line 70, which is consistent with the PR summary. Make sure all calls to this function have been updated accordingly.- for chunk in self.client.generate_stream(prompt, **params): - completion += chunk.text + for chunk in self.client.generate_stream(prompt, **params): + completion += chunk.template
- 76-81: No significant changes were made in this hunk. The code remains the same as before.
pandasai/prompts/base.py (3)
1-16: The
Promptclass has been renamed toAbstractPromptand now inherits from theABC(Abstract Base Class) module. This is a good practice for classes that are meant to be used as base classes, as it makes it clear that they should not be instantiated directly. Thetextattribute has been removed, which suggests that subclasses will need to provide their own implementation of the prompt text or template.48-65: The
to_stringmethod no longer raises aMethodNotImplementedErrorif thetemplateattribute isNone. Instead, it seems to rely on theabstractmethoddecorator on thetemplateproperty to ensure that subclasses provide an implementation. This is a cleaner approach, as it leverages Python's built-in support for abstract methods.68-90: A new subclass
FileBasedPrompthas been introduced, which reads the prompt template from a file. The path to the template file is stored in the_path_to_templateattribute, and thetemplateproperty reads the file content when accessed. This is a good design choice, as it allows for more flexible and maintainable prompts by separating the template content from the code. However, there is no error handling for potential issues with reading the file, such as permission errors or encoding issues. Consider adding a try-except block around the file reading operation to handle these cases gracefully.</blockquote></details></blockquote></details> </details>
* (refactor): change exception handling when reading temaplte content from a file
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- pandasai/exceptions.py (1 hunks)
- pandasai/prompts/base.py (2 hunks)
Additional comments (Suppressed): 4
pandasai/prompts/base.py (4)
1-17: The
Promptclass has been renamed toAbstractPromptand now inherits from theABC(Abstract Base Class) module. This change enforces that any class inheriting fromAbstractPromptmust implement thetemplateproperty, making it more explicit what methods or properties a child class should implement. Thetextattribute has been removed and replaced by the abstracttemplateproperty.54-55: The
templateproperty is declared as an abstract method, which means any class that extendsAbstractPromptmust provide an implementation for this property. This is a good practice as it makes the interface of the base class clear and ensures that all subclasses adhere to the same interface.62-66: The
to_stringmethod now uses thetemplateproperty instead of thetextattribute to format the string representation of the prompt. This change aligns with the removal of thetextattribute and the introduction of thetemplateproperty. Ensure that all subclasses ofAbstractPromptcorrectly implement thetemplateproperty.69-95: A new subclass
FileBasedPrompthas been introduced, which extendsAbstractPrompt. This class is designed for prompts that read template content from files. It introduces a_path_to_templateattribute and overrides thetemplateproperty to read the template content from the file specified by_path_to_template. Error handling has been added to raise aTemplateFileNotFoundErrorif the template file cannot be found, and aRuntimeErrorif there's an issue reading the file. This is a good practice as it separates the concerns of different types of prompts and improves error handling.
* (docs): fix docstring for custom exception
|
Great job @nautics889, way better and more maintainable! Thanks a lot, merging! |
This commit contains refactoring updates related to #593.
Brief summary:
Prompt->AbstractPromptAbstractPromptnow should implement propertytemplate(beforetextattribute had the same sense, but tempalte is more accurate IMHO for this case)FileBasedPrompt(extendsAbstractPrompt)CorrectErrorPromptandGeneratePythonCodePromptto assets/prompt-templates/correct_error_prompt.tmpl and assets/prompt-templates/generate_python_code.tmpl respectivelyCorrectErrorPromptandGeneratePythonCodePromptnow extendFileBasedPrompttextrename totemplate(since it's essentially is a template)Summary by CodeRabbit
AbstractPromptandFileBasedPromptclasses to support a more flexible prompt system, including file-based prompts.Promptclass withAbstractPromptacross the codebase for consistency.TemplateFileNotFoundErrorandLibraryImportErrorfor better error handling and reporting.AbstractPromptclass.