-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(prompt_id): attribute missing error #1509
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.
👍 Looks good to me! Reviewed everything up to 967a15f in 19 seconds
More details
- Looked at
103
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. tests/unit_tests/core/code_generation/test_code_cleaning.py:211
- Draft comment:
Thelast_prompt_id
is set in the context fortest_handle_charts_save_charts_false
, but it is not used in the logic whensave_charts
is False. Consider removing it for clarity. - Reason this comment was not posted:
Confidence changes required:50%
The PR changes the attribute fromprompt_id
tolast_prompt_id
in the_handle_charts
method. This change is reflected in the testtest_handle_charts_save_charts_true
, but the testtest_handle_charts_save_charts_false
still useslast_prompt_id
even though it should not affect the logic whensave_charts
is False. The test logic is correct, but the context setup is unnecessary forlast_prompt_id
.
2. tests/unit_tests/core/code_generation/test_code_cleaning.py:180
- Draft comment:
The word 'reclaration' in the docstring should be corrected to 're-declaration'. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_JIx7M9yjxzDQ23ps
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on 8fc1efa in 32 seconds
More details
- Looked at
138
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. pandasai/core/response/__init__.py:1
- Draft comment:
Reordering imports is generally a style preference. Ensure this aligns with the project's style guide or serves a specific purpose. - Reason this comment was not posted:
Confidence changes required:20%
The import statements in the__init__.py
file have been reordered. This change is not necessary unless it serves a specific purpose, such as grouping related imports together or following a style guide. However, it doesn't affect functionality.
2. pandasai/core/response/base.py:2
- Draft comment:
The import order was changed. Ensure this aligns with the project's style guide. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forAny
inbase.py
was moved. This change is minor and doesn't affect functionality, but it should be consistent with the project's style guide.
3. pandasai/core/response/chart.py:3
- Draft comment:
The import order was changed. Ensure this aligns with the project's style guide. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forAny
inchart.py
was moved. This change is minor and doesn't affect functionality, but it should be consistent with the project's style guide.
4. pandasai/dataframe/base.py:15
- Draft comment:
The import order was changed. Ensure this aligns with the project's style guide. - Reason this comment was not posted:
Confidence changes required:10%
The import statement forBaseResponse
inbase.py
was moved. This change is minor and doesn't affect functionality, but it should be consistent with the project's style guide.
Workflow ID: wflow_bEqhoJESFbjpjifR
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/v3 #1509 +/- ##
==============================================
- Coverage 79.09% 73.45% -5.64%
==============================================
Files 149 68 -81
Lines 6013 2223 -3790
==============================================
- Hits 4756 1633 -3123
+ Misses 1257 590 -667
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Important
Fix missing attribute error in
code_cleaning.py
by replacingprompt_id
withlast_prompt_id
and add tests for_handle_charts()
.code_cleaning.py
, replaceself.context.prompt_id
withself.context.last_prompt_id
in_handle_charts()
to fix missing attribute error.__init__.py
,base.py
,chart.py
,dataframe.py
, andparser.py
for consistency._handle_charts()
intest_code_cleaning.py
to verify behavior whensave_charts
is true or false, and when code is empty or lacks PNG references.This description was created by for 8fc1efa. It will automatically update as commits are pushed.