🧑💻 Make iterator error handler optional and pin poetry-publish#124
🧑💻 Make iterator error handler optional and pin poetry-publish#124
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR tightens type annotations in the itertools helpers by making error handler parameters explicitly Optional and pins the poetry-publish GitHub Action to a specific commit for safer, reproducible publishing. Class diagram for updated itertools tools type annotationsclassDiagram
class tools_module {
+dropwhile(predicate, iterable, handler Optional_ErrorHandler)
+reduce(function, sequence, initial, handler Optional_ErrorHandler)
}
class ErrorHandler {
}
tools_module ..> ErrorHandler : uses
class Optional_ErrorHandler {
}
Optional_ErrorHandler o--> ErrorHandler : wraps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Now that
handleris explicitlyOptional[ErrorHandler], consider also adding concrete type hints forpredicate,iterable/sequence, andfunctionindropwhileandreduceto improve static checking and clarify expected call signatures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `handler` is explicitly `Optional[ErrorHandler]`, consider also adding concrete type hints for `predicate`, `iterable`/`sequence`, and `function` in `dropwhile` and `reduce` to improve static checking and clarify expected call signatures.
## Individual Comments
### Comment 1
<location path="pystreamapi/_itertools/tools.py" line_range="27" />
<code_context>
-def reduce(function, sequence, initial=_initial_missing, handler: ErrorHandler = None):
+def reduce(function, sequence, initial=_initial_missing, handler: Optional[ErrorHandler] = None):
"""
Apply a function of two arguments cumulatively to the items of a sequence
</code_context>
<issue_to_address>
**suggestion:** Consider giving `initial` a more precise type to improve type checking around `reduce`.
Now that `handler` is typed, it’d be helpful to also give `initial` a more precise type instead of relying on the untyped `_initial_missing` sentinel. For example, introduce an accumulator `TypeVar` and use `initial: Union[_Missing, AccumulatorType] = _initial_missing` (or a `Sentinel` protocol) so static type checkers can infer `reduce`’s return type more accurately and avoid `Any` propagation.
Suggested implementation:
```python
def dropwhile(predicate, iterable, handler: Optional[ErrorHandler] = None):
"""
Drop items from the iterable while predicate(item) is true.
Afterward, return every element until the iterable is exhausted.
class _InitialMissing:
"""Sentinel type used to detect when no initial accumulator is provided."""
__slots__ = ()
_initial_missing = _InitialMissing()
AccumulatorT = TypeVar("AccumulatorT")
def reduce(function, sequence, initial: Union[_InitialMissing, AccumulatorT] = _initial_missing, handler: Optional[ErrorHandler] = None) -> AccumulatorT:
```
To fully type this change you should also:
1. Ensure the necessary typing imports are present at the top of `pystreamapi/_itertools/tools.py`, e.g.:
- Add `TypeVar` and `Union` to the existing `typing` imports:
`from typing import Optional, TypeVar, Union`
2. Optionally, if the rest of the `reduce` signature is already typed elsewhere in the file, align `function` and `sequence` parameter types with existing `TypeVar`s (e.g., `Iterable[T]`, `Callable[[AccumulatorT, T], AccumulatorT]`) so that static type checkers can fully infer `AccumulatorT`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
|
|
||
| def reduce(function, sequence, initial=_initial_missing, handler: ErrorHandler = None): | ||
| def reduce(function, sequence, initial=_initial_missing, handler: Optional[ErrorHandler] = None): |
There was a problem hiding this comment.
suggestion: Consider giving initial a more precise type to improve type checking around reduce.
Now that handler is typed, it’d be helpful to also give initial a more precise type instead of relying on the untyped _initial_missing sentinel. For example, introduce an accumulator TypeVar and use initial: Union[_Missing, AccumulatorType] = _initial_missing (or a Sentinel protocol) so static type checkers can infer reduce’s return type more accurately and avoid Any propagation.
Suggested implementation:
def dropwhile(predicate, iterable, handler: Optional[ErrorHandler] = None):
"""
Drop items from the iterable while predicate(item) is true.
Afterward, return every element until the iterable is exhausted.
class _InitialMissing:
"""Sentinel type used to detect when no initial accumulator is provided."""
__slots__ = ()
_initial_missing = _InitialMissing()
AccumulatorT = TypeVar("AccumulatorT")
def reduce(function, sequence, initial: Union[_InitialMissing, AccumulatorT] = _initial_missing, handler: Optional[ErrorHandler] = None) -> AccumulatorT:To fully type this change you should also:
- Ensure the necessary typing imports are present at the top of
pystreamapi/_itertools/tools.py, e.g.:- Add
TypeVarandUnionto the existingtypingimports:
from typing import Optional, TypeVar, Union
- Add
- Optionally, if the rest of the
reducesignature is already typed elsewhere in the file, alignfunctionandsequenceparameter types with existingTypeVars (e.g.,Iterable[T],Callable[[AccumulatorT, T], AccumulatorT]) so that static type checkers can fully inferAccumulatorT.



Summary by Sourcery
Clarify error handler typing in iterator utilities and pin the poetry publish GitHub Action to a specific commit for publishing.
Enhancements:
CI: