chore: add generator return type hints to query/llm/text_utils.py#2329
chore: add generator return type hints to query/llm/text_utils.py#2329ilias-laoukili wants to merge 2 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit return type hints for generator utilities in graphrag/query/llm/text_utils.py to improve static type checking in the LLM/RAG pipeline.
Changes:
- Annotate
batched(...)to explicitly return an iterator of batches. - Annotate
chunk_text(...)to explicitly return an iterator of text chunks.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def batched(iterable: Iterator, n: int) -> Iterator[tuple]: | ||
| """ | ||
| Batch data into tuples of length n. The last batch may be shorter. |
There was a problem hiding this comment.
batched is annotated as returning Iterator[tuple], but tuple is a generic type and leaving it un-parameterized loses most of the downstream type-safety (and may trigger "missing type parameters" warnings in stricter mypy/pyright configs). Since this helper preserves element type, consider making it generic (e.g., T = TypeVar('T')) and typing the signature as iterable: Iterable[T] (or Iterator[T]) with a return of Iterator[tuple[T, ...]] to accurately reflect variable-length batches while keeping element types.
|
@microsoft-github-policy-service agree |
Description
Add missing return type hints to two generator functions in
packages/graphrag/graphrag/query/llm/text_utils.py.Related Issues
Closes #2330
Proposed Changes
batched(iterable: Iterator, n: int)→ adds-> Iterator[tuple]chunk_text(text: str, max_tokens: int, tokenizer: Tokenizer | None = None)→ adds-> Iterator[str]Iteratorwas already imported fromcollections.abc; no new imports required.Checklist
Additional Notes
Verified with
mypy packages/graphrag/graphrag/query/llm/text_utils.py --ignore-missing-imports— no issues. The pre-existingimport-not-founderrors in the baseline are caused by uninstalled monorepo sub-packages and are unrelated to this change.