Skip to content

chore: improve select_streaming_callback type hints#9513

Merged
anakin87 merged 1 commit intomainfrom
select-streaming-callback-overload
Jun 13, 2025
Merged

chore: improve select_streaming_callback type hints#9513
anakin87 merged 1 commit intomainfrom
select-streaming-callback-overload

Conversation

@anakin87
Copy link
Copy Markdown
Member

@anakin87 anakin87 commented Jun 13, 2025 •
edited
Loading

Proposed Changes:

  • improve select_streaming_callback type hints using overload
  • this allows us to specify more precise type hints for some other methods, clarifying their purpose and removing some type: ignore comments

How did you test it?

CI

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@github-actions github-actions Bot added the type:documentation Improvements on the docs label Jun 13, 2025
@anakin87 anakin87 added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Jun 13, 2025
@anakin87 anakin87 marked this pull request as ready for review June 13, 2025 10:23
@anakin87 anakin87 requested a review from a team as a code owner June 13, 2025 10:23
@anakin87 anakin87 requested review from sjrl and removed request for a team June 13, 2025 10:23
@anakin87 anakin87 self-assigned this Jun 13, 2025
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Jun 13, 2025

Pull Request Test Coverage Report for Build 15632259890

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 45 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+0.004%) to 90.281%
Files with Coverage Reduction New Missed Lines %
components/generators/hugging_face_local.py 2 92.05%
components/generators/hugging_face_api.py 3 96.34%
dataclasses/streaming_chunk.py 4 91.07%
components/generators/chat/openai.py 5 95.76%
components/generators/chat/hugging_face_local.py 7 83.63%
components/generators/chat/hugging_face_api.py 8 95.6%
utils/hf.py 16 86.77%
Totals Coverage Status
Change from base Build 15588214352: 0.004%
Covered Lines: 11556
Relevant Lines: 12800
💛 - Coveralls

sjrl
sjrl reviewed Jun 13, 2025
View reviewed changes
Comment thread haystack/components/generators/chat/hugging_face_local.py

from haystack import component, default_from_dict, default_to_dict, logging
from haystack.dataclasses import ChatMessage, ComponentInfo, StreamingCallbackT, ToolCall
from haystack.dataclasses import AsyncStreamingCallbackT, ChatMessage, ComponentInfo, StreamingCallbackT, ToolCall
Copy link
Copy Markdown
Contributor

@sjrl sjrl Jun 13, 2025

Choose a reason for hiding this comment

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

as a heads up I did a refactor of this component in #9455 so I'd appreciate if we could leave out the changes here and I can add them into mine once this PR is merged

Copy link
Copy Markdown
Contributor

@sjrl sjrl Jun 13, 2025

Choose a reason for hiding this comment

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

Hmm actually fine to leave. I'll handle the merge conflict rather after this PR is merged

sjrl
sjrl approved these changes Jun 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@sjrl sjrl 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!

@anakin87 anakin87 merged commit 580683b into main Jun 13, 2025
26 of 27 checks passed
@anakin87 anakin87 deleted the select-streaming-callback-overload branch June 13, 2025 12:24
@anakin87 anakin87 mentioned this pull request Jun 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjrl sjrl sjrl approved these changes

Assignees

@anakin87 anakin87

Labels

ignore-for-release-notes PRs with this flag won't be included in the release notes. type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@anakin87 @coveralls @sjrl