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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(batch-classification,extract-table): simplify code, improve functionalities, introduce langsmith library #442

Merged
merged 4 commits into from
Feb 18, 2024

Conversation

jxnl
Copy link
Owner

@jxnl jxnl commented Feb 18, 2024

Ellipsis 馃殌 This PR description was created by Ellipsis for commit 5709a4a.

Summary:

This PR refactors and simplifies code in batch-classification and extract-table, improves functionalities, introduces the langsmith library in new files for function call tracing, and removes unnecessary imports and functions.

Key points:

  • Refactored and simplified code in run-cache.py and run.py in batch-classification.
  • Improved table extraction in run_vision.py in extract-table.
  • Introduced langsmith library in new files run_langsmith.py in batch-classification and run_vision_langsmith.py in extract-table.
  • Removed unnecessary imports and functions in several files.

Generated with 鉂わ笍 by ellipsis.dev

@ellipsis-dev ellipsis-dev bot changed the title ... refactor(batch-classification,extract-table): simplify code, improve functionalities, introduce langsmith library Feb 18, 2024
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 entire PR up to commit 8637ba8

Reviewed 670 lines of code across 5 files in 1 minute(s) and 58 second(s).

See details
  • Skipped files: 0
  • Confidence threshold: 85%
  • Drafted 8 additional comments.
  • Workflow ID: wflow_bLqmf4lPS8QjTngZ
View 8 draft comments

These comments were drafted by Ellipsis, but were filtered out of the final review. They're included here so you can see our internal thought process and help you configure your ellipsis.yaml.

Drafted 8 comments under confidence threshold

Filtered comment at examples/batch-classification/run-cache.py:1

Notes: The PR author has removed the caching functionality from the run-cache.py file. This could potentially impact the performance of the application if the function classify_question is called multiple times with the same arguments. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

The caching functionality has been removed. If the function classify_question is called multiple times with the same arguments, this could potentially impact performance. Please ensure this change is intentional and justified.

Confidence changes required: 50%

Filtered comment at examples/batch-classification/run-cache.py:14

Notes: The PR author has removed some question types from the QuestionType Enum in both run-cache.py and run.py files. This could potentially cause issues if other parts of the codebase are expecting these removed question types. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

Some question types have been removed from the QuestionType Enum. Please ensure this change is intentional and won't cause issues in other parts of the codebase that might be expecting these removed question types.

Confidence changes required: 50%

Filtered comment at examples/batch-classification/run-cache.py:41

Notes: The PR author has added a new field chain_of_thought to the QuestionClassification model in all three files. This change seems fine as long as it's intentional and the new field is being used somewhere. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

A new field chain_of_thought has been added to the QuestionClassification model. Please ensure this change is intentional and the new field is being used as expected.

Confidence changes required: 50%

Filtered comment at examples/batch-classification/run-cache.py:57

Notes: The PR author has modified the classify function in all three files. The changes seem fine as long as they are intentional and the function is still working as expected. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

The classify function has been modified. Please ensure this change is intentional and the function is still working as expected.

Confidence changes required: 50%

Filtered comment at examples/batch-classification/run-cache.py:72

Notes: The PR author has modified the main function in all three files. The changes seem fine as long as they are intentional and the function is still working as expected. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

The main function has been modified. Please ensure this change is intentional and the function is still working as expected.

Confidence changes required: 50%

Filtered comment at examples/batch-classification/run_langsmith.py:1

Notes: The PR author has added a new file run_langsmith.py in the batch-classification directory. This file seems to be a variant of the other two files in the same directory but uses the langsmith library. The changes in this file seem fine as long as they are intentional and the new file is being used somewhere. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

A new file run_langsmith.py has been added. Please ensure this change is intentional and the new file is being used as expected.

Confidence changes required: 50%

Filtered comment at examples/extract-table/run_vision.py:75

Notes: The PR author has modified the extract function in the run_vision.py file. The changes seem fine as long as they are intentional and the function is still working as expected. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

The extract function has been modified. Please ensure this change is intentional and the function is still working as expected.

Confidence changes required: 50%

Filtered comment at examples/extract-table/run_vision_langsmith.py:1

Notes: The PR author has added a new file run_vision_langsmith.py in the extract-table directory. This file seems to be a variant of the run_vision.py file but uses the langsmith library. The changes in this file seem fine as long as they are intentional and the new file is being used somewhere. However, without knowing the context or the reason behind this change, it's hard to say if this is a problem. The PR description doesn't provide any information about this.

A new file run_vision_langsmith.py has been added. Please ensure this change is intentional and the new file is being used as expected.

Confidence changes required: 50%


Something look wrong? You can customize Ellipsis by editing the ellipsis.yaml for this repository.

Generated with 鉂わ笍 by ellipsis.dev

@jxnl jxnl merged commit 5709a4a into main Feb 18, 2024
12 checks passed
@jxnl jxnl deleted the langsmith branch February 18, 2024 16:50
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! Incremental review on commit 5709a4a

Reviewed 669 lines of code across 5 files in 2 minute(s) and 40 second(s).

See details
  • Skipped files: 0
  • Confidence threshold: 85%
  • Drafted 11 additional comments.
  • Workflow ID: wflow_rGZCNGmJDnxKz7ys
View 11 draft comments

These comments were drafted by Ellipsis, but were filtered out of the final review. They're included here so you can see our internal thought process and help you configure your ellipsis.yaml.

Drafted 11 additional comments

Filtered comment at examples/batch-classification/run_langsmith.py:5

Notes: The PR introduces the use of the langsmith library for tracing function calls in the run_vision_langsmith.py and run_langsmith.py files. This is a good practice for debugging and understanding the flow of the program. However, it's important to ensure that the library is included in the project dependencies to avoid any runtime errors.

Ensure that the langsmith library is included in the project dependencies.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/extract-table/run_vision_langsmith.py:14

Notes: The PR introduces the use of the langsmith library for tracing function calls in the run_vision_langsmith.py and run_langsmith.py files. This is a good practice for debugging and understanding the flow of the program. However, it's important to ensure that the library is included in the project dependencies to avoid any runtime errors.

Ensure that the langsmith library is included in the project dependencies.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run.py:42

Notes: The PR introduces a new field chain_of_thought in the QuestionClassification model in the run.py, run-cache.py and run_langsmith.py files. This is a good practice for providing more context about the classification. However, it's important to ensure that this field is properly handled in the rest of the codebase to avoid any runtime errors.

Ensure that the new field chain_of_thought is properly handled in the rest of the codebase.

Confidence changes required: 100%

Score: 70%

Filtered comment at examples/batch-classification/run-cache.py:41

Notes: The PR introduces a new field chain_of_thought in the QuestionClassification model in the run.py, run-cache.py and run_langsmith.py files. This is a good practice for providing more context about the classification. However, it's important to ensure that this field is properly handled in the rest of the codebase to avoid any runtime errors.

Ensure that the new field chain_of_thought is properly handled in the rest of the codebase.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run_langsmith.py:44

Notes: The PR introduces a new field chain_of_thought in the QuestionClassification model in the run.py, run-cache.py and run_langsmith.py files. This is a good practice for providing more context about the classification. However, it's important to ensure that this field is properly handled in the rest of the codebase to avoid any runtime errors.

Ensure that the new field chain_of_thought is properly handled in the rest of the codebase.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run.py:57

Notes: The PR refactors the classify function in the run.py, run-cache.py and run_langsmith.py files. The refactoring seems to be done correctly, but it's important to ensure that the function still behaves as expected in all scenarios.

Ensure that the refactored classify function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run-cache.py:57

Notes: The PR refactors the classify function in the run.py, run-cache.py and run_langsmith.py files. The refactoring seems to be done correctly, but it's important to ensure that the function still behaves as expected in all scenarios.

Ensure that the refactored classify function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run_langsmith.py:61

Notes: The PR refactors the classify function in the run.py, run-cache.py and run_langsmith.py files. The refactoring seems to be done correctly, but it's important to ensure that the function still behaves as expected in all scenarios.

Ensure that the refactored classify function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/batch-classification/run-cache.py:72

Notes: The PR refactors the main function in the run.py and run-cache.py files. The refactoring seems to be done correctly, but it's important to ensure that the function still behaves as expected in all scenarios.

Ensure that the refactored main function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/extract-table/run_vision.py:75

Notes: The PR introduces a new function extract in the run_vision.py and run_vision_langsmith.py files. This function seems to be correctly implemented, but it's important to ensure that it behaves as expected in all scenarios.

Ensure that the new extract function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%

Filtered comment at examples/extract-table/run_vision_langsmith.py:78

Notes: The PR introduces a new function extract in the run_vision.py and run_vision_langsmith.py files. This function seems to be correctly implemented, but it's important to ensure that it behaves as expected in all scenarios.

Ensure that the new extract function behaves as expected in all scenarios.

Confidence changes required: 100%

Score: 0%


Something look wrong? You can customize Ellipsis by editing the ellipsis.yaml for this repository.

Generated with 鉂わ笍 by ellipsis.dev

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.

None yet

1 participant