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

Passing Functions as Tools #321

Merged
merged 32 commits into from
Nov 20, 2024
Merged

Passing Functions as Tools #321

merged 32 commits into from
Nov 20, 2024

Conversation

ParthSareen
Copy link
Contributor

@ParthSareen ParthSareen commented Nov 11, 2024

Features:

  • Pass in python functions as tools into .chat(tools=[python_func,...]) 🥳
  • Follows Google Python Docstring Styleguide - See Google Docstrings
  • Lenient on docstring specification - should handle unusual cases elegantly
  • Can still use a mixture of Python functions as tools and JSON values - feature is purely additive.

Examples to come on follow up for both Pydantic and Functions as Tools

@ParthSareen ParthSareen force-pushed the parth/tool-parsing branch 2 times, most recently from 4e8bd4b to 4fcdf70 Compare November 11, 2024 23:52
ollama/_types.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
@ParthSareen ParthSareen requested a review from mxyng November 12, 2024 00:47
@ParthSareen ParthSareen marked this pull request as ready for review November 12, 2024 00:47
ollama/_types.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
@ParthSareen
Copy link
Contributor Author

ParthSareen commented Nov 12, 2024

Blocked on ollama/ollama#7613 to support typing options

ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_types.py Outdated Show resolved Hide resolved
ollama/_types.py Outdated Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
ollama/_client.py Outdated Show resolved Hide resolved
ollama/_client.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Show resolved Hide resolved
ollama/_client.py Outdated Show resolved Hide resolved
ollama/_client.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
tests/test_client.py Outdated Show resolved Hide resolved
# Test missing required fields
incomplete_tool = {
'type': 'function',
'function': {'name': 'test'}, # missing description and parameters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is parameters required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So in the previous behavior - no.
But since the last Pydantic PR you have to provide Parameters. Tested out both the OpenAI Python SDK with Ollama and the current published version of ollama-python. I think it might be worthwhile to have the Tool class have optional on all params to make sure it is backwards compatible.

Seems like function calling still works with not a fully-fledged JSON (I'm sure it's flakey but not our job).

Proposed Tool def'n:

class Tool(SubscriptableBaseModel):
  type: Optional[Literal['function']] = 'function'

  class Function(SubscriptableBaseModel):
    name: Optional[str] = None
    description: Optional[str] = None

    class Parameters(SubscriptableBaseModel):
      type: Optional[Literal['object']] = 'object'
      required: Optional[Sequence[str]] = None

      class Property(SubscriptableBaseModel):
        model_config = ConfigDict(arbitrary_types_allowed=True)

        type: Optional[str] = None
        description: Optional[str] = None

      properties: Optional[Mapping[str, Property]] = None

    parameters: Optional[Parameters] = None

  function: Optional[Function] = None

ollama/_client.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
ollama/_utils.py Outdated Show resolved Hide resolved
# 1. A parenthetical expression like (integer) - captured in group 1
# 2. A colon :
# Followed by optional whitespace. Only split on first occurrence.
parts = re.split(r'(?:\(([^)]*)\)|:)\s*', line, maxsplit=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tend to avoid regexp when possible since it's hard to grok. In this scenario, a simpler solution would be to split on the mandatory : than parse the pre-colon and post-colon sections independently. Here's an example that passes your tests

  for line in parsed_docstring['args'].splitlines():
    pre, _, post = line.partition(':')
    if not pre.strip():
      continue
    if not post.strip() and last_key:
      parsed_docstring[last_key] += ' ' + pre
      continue

    arg_name, _, _ = pre.replace('(', ' ').partition(' ')
    last_key = arg_name.strip()

    parsed_docstring[last_key] = post.strip()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO @ParthSareen to spin out issue

types = {t.get('type', 'string') for t in v.get('anyOf')} if 'anyOf' in v else {v.get('type', 'string')}
if 'null' in types:
schema['required'].remove(k)
types.discard('null')
Copy link
Collaborator

Choose a reason for hiding this comment

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

If null is the only type (for some reason), this will be an empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is okay, IMO something like:

def (a:None, b:type(None)):
  ...

is extremely unlikely

* Fix image serialization
@ParthSareen ParthSareen merged commit 139c89e into main Nov 20, 2024
6 checks passed
@ParthSareen ParthSareen deleted the parth/tool-parsing branch November 20, 2024 23:49
pressdarling pushed a commit to pressdarling/ollama-python that referenced this pull request Dec 1, 2024
* Functions can now be passed as tools
pressdarling pushed a commit to pressdarling/ollama-python that referenced this pull request Dec 30, 2024
* Functions can now be passed as tools
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