feat: add response_format param to OllamaChatGenerator
#1282
Conversation
github-actions
bot
added
integration:ollama
type:documentation
|
Hey! |
| @@ -26,7 +26,7 @@ classifiers = [ | |||
| "Programming Language :: Python :: Implementation :: CPython", | |||
| "Programming Language :: Python :: Implementation :: PyPy", | |||
| ] | |||
| dependencies = ["haystack-ai", "ollama>=0.4.0"] | |||
| dependencies = ["haystack-ai", "ollama>=0.4.0", "pydantic>=2.0.0"] | |||
There was a problem hiding this comment.
Ollama python client already depends on pydantic, no? I would not add a direct dependency if not required.
No, it might be better to include it. I only think it's not necessary to pin it because I assume it is already pinned in the ollama python client.
| @@ -68,7 +68,7 @@ python = ["3.9", "3.10", "3.11", "3.12"] | |||
| [tool.hatch.envs.lint] | |||
| installer = "uv" | |||
| detached = true | |||
| dependencies = ["pip", "black>=23.1.0", "mypy>=1.0.0", "ruff>=0.0.243"] | |||
| dependencies = ["pip", "black>=23.1.0", "mypy>=1.0.0", "ruff>=0.0.243", "pydantic>=2.0.0"] | |||
There was a problem hiding this comment.
why is pydantic needed in the linting environment?
There was a problem hiding this comment.
Hmm, you are right. It could have been helpful if in a test mypy needs to verify that a dictionary structure is valid according to the JsonSchemaValue type definition but I guess we don't need that.
| @@ -40,6 +41,7 @@ def __init__( | |||
| timeout: int = 120, | |||
| keep_alive: Optional[Union[float, str]] = None, | |||
| streaming_callback: Optional[Callable[[StreamingChunk], None]] = None, | |||
| response_format: Optional[Union[Literal["", "json"], JsonSchemaValue]] = None, | |||
There was a problem hiding this comment.
I understand that the type hint comes from the Ollama python client, but it is confusing to me.
I would change it to something more readable, like response_format: Union[None, Literal['json'], JsonSchemaValue] = None
- default is None (TODO: check if it has the same meaning as an empty string)
- you can pass "json" if you only want to ensure that the output format is a valid JSON
JsonSchemaValuecorresponds to Json Schemas, passed as dictionaries
I would add a similar but more refined explanation to the response_format docstring above.
Related Issues
Proposed Changes:
Add a new param
response_formatto the OllamaChatGenerator to support structured outputs.How did you test it?
Updated the tests
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.