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

chore: use modern type annotation syntax #419

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akx
Copy link
Contributor

@akx akx commented Jan 14, 2025

Using from __future__ import annotations makes futuristic annotations usable in any Python version.

This adds eval_type_backport as a dep for older Pythons so Pydantic can robustly evaluate these annotations.

@ParthSareen
Copy link
Contributor

I believe I've tried this out before but it breaks the parsing logic when passing tools as function - could you verify?

@akx
Copy link
Contributor Author

akx commented Jan 16, 2025

I believe I've tried this out before but it breaks the parsing logic when passing tools as function - could you verify?

Wouldn't there be a failing test here if that was the case? 😁

@ParthSareen
Copy link
Contributor

@akx I would hope so HAHA. I'm pretty sure I had a bunch of tests for parsing logic but maybe it's fine. I'd still want to just run the example for a sanity check.

@akx akx force-pushed the futuristic-types-py38 branch from 7f896f0 to 1fb7b5f Compare January 16, 2025 07:01
@akx
Copy link
Contributor Author

akx commented Jan 16, 2025

@ParthSareen Yes, the example still works on various versions of Python. Love that it's nondeterministic though and sometimes the model decides to pass in strings instead of ints, and sometimes it apologizes about a confusing response.

This will require #422 to be merged first so CI can be green at all.

(ollama-python) ~/b/ollama-python (futuristic-types-py38 *) $ tox run
py38: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/9/ollama-0.0.0.tar.gz
py38: commands[0]> python --version
Python 3.8.19
py38: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': '3', 'b': '1'}
Function output: 31
Final response: The answer to the equation 3 + 1 is 4.
py38: OK ✔ in 4.78 seconds
py39: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/10/ollama-0.0.0.tar.gz
py39: commands[0]> python --version
Python 3.9.6
py39: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': 3, 'b': 1}
Function output: 4
Final response: The result of the calculation is 4.
py39: OK ✔ in 3.32 seconds
py310: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/11/ollama-0.0.0.tar.gz
py310: commands[0]> python --version
Python 3.10.16
py310: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': 3, 'b': 1}
Function output: 4
Final response: The answer to the math problem "three plus one" is 4.
py310: OK ✔ in 2.6 seconds
py311: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/12/ollama-0.0.0.tar.gz
py311: commands[0]> python --version
Python 3.11.11
py311: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': 3, 'b': 1}
Function output: 4
Final response: I apologize for the confusing response earlier. The correct answer to the question "What is three plus one?" is indeed 4.
py311: OK ✔ in 2.81 seconds
py313: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/13/ollama-0.0.0.tar.gz
py313: commands[0]> python --version
Python 3.13.1
py313: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': '3', 'b': '1'}
Function output: 31
Final response: The answer to three plus one is 4.

(ollama-python) ~/b/ollama-python (futuristic-types-py38 *) $ tox run
py38: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/14/ollama-0.0.0.tar.gz
py38: commands[0]> python --version
Python 3.8.19
py38: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': 3, 'b': 1}
Function output: 4
Final response: The result of the calculation 3 + 1 is indeed 4.
py38: OK ✔ in 3.11 seconds
py39: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/15/ollama-0.0.0.tar.gz
py39: commands[0]> python --version
Python 3.9.6
py39: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': '3', 'b': '1'}
Function output: 31
Final response: The answer to the math problem is 4.
py39: OK ✔ in 3.66 seconds
py310: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/16/ollama-0.0.0.tar.gz
py310: commands[0]> python --version
Python 3.10.16
py310: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': '3', 'b': '1'}
Function output: 31
Final response: The answer to 3 + 1 is 4.
py310: OK ✔ in 2.52 seconds
py311: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/17/ollama-0.0.0.tar.gz
py311: commands[0]> python --version
Python 3.11.11
py311: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': '3', 'b': '1'}
Function output: 31
Final response: The answer to the equation 3 + 1 is 4.
py311: OK ✔ in 7.85 seconds
py313: install_package> python -I -m pip install --force-reinstall --no-deps /Users/akx/build/ollama-python/.tox/.tmp/package/18/ollama-0.0.0.tar.gz
py313: commands[0]> python --version
Python 3.13.1
py313: commands[1]> python examples/tools.py
Prompt: What is three plus one?
Calling function: add_two_numbers
Arguments: {'a': 3, 'b': 1}
Function output: 4
Final response: The answer to the equation 3 + 1 is 4.

@ParthSareen
Copy link
Contributor

@akx what model are you using? I find it funny that it does that and also a bit annoying.

Working on sampling in the new engine and so also thinking about improving tool calling as a whole through structured generation. Hopefully can have something soon which can improve this experience.

I'll check both the PRs out in the AM. Really appreciate these especially since I've been heads down on a few other areas!!

@akx
Copy link
Contributor Author

akx commented Jan 16, 2025

@akx what model are you using? I find it funny that it does that and also a bit annoying.

llama3.2. Am on a train, so no chance to pull llama3.1...

@ParthSareen
Copy link
Contributor

ParthSareen commented Jan 17, 2025

@akx we should be able to rebase and get this in :)

Actually - do we need the extra package - could we do without it? We try to keep the package very minimal with low external dependencies

@akx akx force-pushed the futuristic-types-py38 branch from 1fb7b5f to 61e68f3 Compare January 21, 2025 11:24
@akx
Copy link
Contributor Author

akx commented Jan 21, 2025

Actually - do we need the extra package - could we do without it?

It's marked to be installed only when the environment has an old Python.

I can elide it from this library's deps, but then users will need to install it on those versions by hand. 🤷

@ParthSareen
Copy link
Contributor

Actually - do we need the extra package - could we do without it?

It's marked to be installed only when the environment has an old Python.

I can elide it from this library's deps, but then users will need to install it on those versions by hand. 🤷

I'm on the fence for this PR - to a degree I think it is good to have the old typing here as it forces us to think about those cases when working on any typing-related feature. I don't want to bring in unnecessary external dependencies unless it's vastly improving the developer experience. I'll think a bit more about it but honestly this PR improves our experience as devs not really anyone else's.

@akx
Copy link
Contributor Author

akx commented Jan 22, 2025

to a degree I think it is good to have the old typing here as it forces us to think about those cases when working on any typing-related feature.

If you only have old typing here, then you're not thinking about new typing when working on any typing-related features. You can mix and match old and new typing in tests as you like 🤷

I don't want to bring in unnecessary external dependencies unless it's vastly improving the developer experience.

Based on https://pypistats.org/packages/ollama, about 6.5% of users would ever see the new dependency.

Screenshot 2025-01-22 at 8 12 23

@ParthSareen
Copy link
Contributor

to a degree I think it is good to have the old typing here as it forces us to think about those cases when working on any typing-related feature.

If you only have old typing here, then you're not thinking about new typing when working on any typing-related features. You can mix and match old and new typing in tests as you like 🤷

I don't want to bring in unnecessary external dependencies unless it's vastly improving the developer experience.

Based on https://pypistats.org/packages/ollama, about 6.5% of users would ever see the new dependency.

Screenshot 2025-01-22 at 8 12 23

That's a fair point. Going to leave this open for now and come back to this. Thanks!

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