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

fix: rename python tool to ipython for better tooluse format adherence #361

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Dec 22, 2024

Attempt at improving #327

No idea if it actually performs better.


Important

Renames the 'python' tool to 'ipython' for better tool use format adherence, updating initialization and tests accordingly.

  • Behavior:
    • Renames tool from python to ipython in gptme/tools/__init__.py and gptme/tools/python.py.
    • Updates tool initialization logic in init_tools() to sort by ipython.
  • Tests:
    • Updates test_tools_info() in test_util_cli.py to check for ipython instead of python.
    • Updates test_subagent() in test_cli.py to use ipython in tool list.

This description was created by Ellipsis for fd8ea4d. 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 955afab in 13 seconds

More details
  • Looked at 44 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. gptme/tools/__init__.py:79
  • Draft comment:
    The import statement should be updated to reflect the tool name change from 'python' to 'ipython'.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_s6YCea8pKD6G18RS


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

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.99%. Comparing base (cdbfae8) to head (fd8ea4d).

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #361      +/-   ##
==========================================
- Coverage   71.01%   70.99%   -0.02%     
==========================================
  Files          68       68              
  Lines        5417     5417              
==========================================
- Hits         3847     3846       -1     
- Misses       1570     1571       +1     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 69.81% <ø> (ø)
openai/gpt-4o-mini 68.91% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ErikBjare ErikBjare force-pushed the dev/rename-python-tool-to-ipython branch from 955afab to 113fa58 Compare December 22, 2024 13:59
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 113fa58 in 19 seconds

More details
  • Looked at 52 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_v7TPTFwBBZoHjjRW


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

@ErikBjare ErikBjare force-pushed the dev/rename-python-tool-to-ipython branch from 113fa58 to a5a5984 Compare January 4, 2025 18:45
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 a5a5984 in 13 seconds

More details
  • Looked at 52 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. gptme/tools/__init__.py:79
  • Draft comment:
    Update __all__ to reflect the tool name change from python_tool to ipython_tool for consistency and to avoid import issues.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_ETdeo2vvKY4EmoW6


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

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 fd8ea4d in 17 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_cli.py:329
  • Draft comment:
    The test function name test_python should be updated to test_ipython to reflect the change from 'python' to 'ipython'. This applies to test_python_error as well.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_wWWar4Y7x73rkQ3W


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

@ErikBjare
Copy link
Owner Author

Lets give this a shot

@ErikBjare ErikBjare merged commit d4407e3 into master Jan 5, 2025
7 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.

2 participants