refactor: Refactor HuggingFaceLocalChatGenerator
#9455
Conversation
sjrl
changed the title
HFLocalChatGenerator
sjrl
added
the
ignore-for-release-notes
sjrl
added
ignore-for-release-notes
sjrl
changed the title
HFLocalChatGeneratorHuggingFaceLocalChatGenerator
Pull Request Test Coverage Report for Build 15634575323Details
💛 - Coveralls |
| @@ -138,7 +138,7 @@ def __init__( # pylint: disable=too-many-positional-arguments | |||
| tools: Optional[Union[List[Tool], Toolset]] = None, | |||
| tool_parsing_function: Optional[Callable[[str], Optional[List[ToolCall]]]] = None, | |||
| async_executor: Optional[ThreadPoolExecutor] = None, | |||
| ): | |||
| ) -> None: | |||
There was a problem hiding this comment.
For my own knowledge, is this now required by linter? I remember having a similar error recently and we didnt specify return types for init in other components.
There was a problem hiding this comment.
Ahh no I don't think it's required right @anakin87 ? At least not yet. It's more just to start getting used to always adding return types to functions.
There was a problem hiding this comment.
In case there is at least one argument annotated, it's not required. https://mypy.readthedocs.io/en/stable/class_basics.html#annotating-init-methods
| ) | ||
|
|
||
| # We know it's not None because we check it in _prepare_inputs | ||
| assert self.pipeline is not None |
There was a problem hiding this comment.
can we use # type: ignore[misc] to replace this like we do in run_async
There was a problem hiding this comment.
True, but I also think it's fine to leave the assert. It's what @anakin87 and others have been doing else where in the code to help mypy
Related Issues
Proposed Changes:
HuggingFaceLocalChatGeneratorto reuse code where possible between therunandrun_asyncmethods.How did you test it?
Existing tests
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.