Conversation
… thrown in other threads
There was a problem hiding this comment.
Pull Request Overview
This PR adds type stubs (.pyi file) for the streamerate library to provide type annotations for better IDE support and static type checking. The stub file defines type signatures for all public classes and methods in the streamerate package.
Key changes:
- Created
streamerate/__init__.pyiwith complete type annotations - Defined type signatures for the main classes:
stream,slist,sset, andsdict - Added proper type variable definitions and method overloads for complex method signatures
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def keyBy(self, keyfunc: Callable[[_K], _V]) -> stream[Tuple[_K, _V]]: ... | ||
| @overload | ||
| def keyBy(self) -> stream[Tuple[_K, _K]]: ... | ||
| def keyBy(self, keyfunc: Callable[[_K], _V] = ...) -> stream[Tuple[_K, _V]]: ... |
There was a problem hiding this comment.
The return type is incorrect. When keyfunc is provided, it maps _K to _V and returns Tuple[_V, _K] (key, value), not Tuple[_K, _V]. Based on the implementation at line 716 of streams.py which does (keyfunc(h), h), the key comes first. Change the return type to stream[Tuple[_V, _K]].
| def keyBy(self, keyfunc: Callable[[_K], _V]) -> stream[Tuple[_K, _V]]: ... | |
| @overload | |
| def keyBy(self) -> stream[Tuple[_K, _K]]: ... | |
| def keyBy(self, keyfunc: Callable[[_K], _V] = ...) -> stream[Tuple[_K, _V]]: ... | |
| def keyBy(self, keyfunc: Callable[[_K], _V]) -> stream[Tuple[_V, _K]]: ... | |
| @overload | |
| def keyBy(self) -> stream[Tuple[_K, _K]]: ... | |
| def keyBy(self, keyfunc: Callable[[_K], _V] = ...) -> stream[Tuple[_V, _K]]: ... |
There was a problem hiding this comment.
right, there is a wrong inline type hint then in sdict
| @overload | ||
| def keyBy(self, keyfunc: Callable[[_K], _V]) -> stream[Tuple[_K, _V]]: ... | ||
| @overload | ||
| def keyBy(self) -> stream[Tuple[_K, _K]]: ... |
There was a problem hiding this comment.
The return type is incorrect. When keyfunc is omitted (using identity function), the method returns Tuple[_K, _K] where both elements are the same value. However, the non-overloaded signature at line 116 should use the same type variable pattern as line 113. This signature should not exist as a separate overload since the default parameter handling covers this case.
| def keyBy(self) -> stream[Tuple[_K, _K]]: ... | |
| # Removed redundant overload for keyBy with no arguments |
There was a problem hiding this comment.
yep, I think redundant
| @overload | ||
| def min(self) -> _K: ... | ||
| @overload | ||
| def min(self, key: Callable[[_K], _V]) -> _V: ... |
There was a problem hiding this comment.
The return type is incorrect. The built-in min() function with a key parameter returns the minimum element from the iterable, not the minimum key value. When a key function is provided, it should return _K, not _V. The key function is only used for comparison purposes.
| def min(self, key: Callable[[_K], _V]) -> _V: ... | |
| def min(self, key: Callable[[_K], _V]) -> _K: ... |
| @overload | ||
| def max(self) -> _K: ... | ||
| @overload | ||
| def max(self, key: Callable[[_K], _V]) -> _V: ... |
There was a problem hiding this comment.
The return type is incorrect. The built-in max() function with a key parameter returns the maximum element from the iterable, not the maximum key value. When a key function is provided, it should return _K, not _V. The key function is only used for comparison purposes.
| def max(self, key: Callable[[_K], _V]) -> _V: ... | |
| def max(self, key: Callable[[_K], _V]) -> _K: ... |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a comprehensive public type stub Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MapWorker
participant _MapException
Note right of MapWorker: mapping function runs and raises
Caller->>MapWorker: submit element for mapping
MapWorker-->>_MapException: capture exc_info and wrap
MapWorker-->>Caller: propagate wrapper
Caller->>_MapException: call get_adjusted_exception()
_MapException-->>Caller: returns exception reconstructed with original traceback
Note over Caller,_MapException: re-raise uses reconstructed exception to preserve traceback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
streamerate/__init__.pyi(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (windows-latest, 3.8)
- GitHub Check: build (macos-latest, 3.8)
- GitHub Check: build (windows-latest, 3.13)
| def toSet(self) -> sset[_K]: ... | ||
| def toList(self) -> slist[_K]: ... | ||
| def sorted(self, key: Any = None, reverse: bool = False) -> slist[_K]: ... | ||
| def toMap(self: stream[Tuple[_T, _V]]) -> sdict[_T, _V]: ... |
There was a problem hiding this comment.
Restore the cmp parameter on sorted.
The runtime API still accepts a cmp callable, so omitting it here makes type checkers reject valid calls. Please add the parameter back in the stub.
Apply this diff:
- def sorted(self, key: Any = None, reverse: bool = False) -> slist[_K]: ...
+ def sorted(
+ self,
+ key: Optional[Callable[[_K], Any]] = None,
+ cmp: Optional[Callable[[_K, _K], int]] = None,
+ reverse: bool = False,
+ ) -> slist[_K]: ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def toSet(self) -> sset[_K]: ... | |
| def toList(self) -> slist[_K]: ... | |
| def sorted(self, key: Any = None, reverse: bool = False) -> slist[_K]: ... | |
| def toMap(self: stream[Tuple[_T, _V]]) -> sdict[_T, _V]: ... | |
| def toSet(self) -> sset[_K]: ... | |
| def toList(self) -> slist[_K]: ... | |
| def sorted( | |
| self, | |
| key: Optional[Callable[[_K], Any]] = None, | |
| cmp: Optional[Callable[[_K, _K], int]] = None, | |
| reverse: bool = False, | |
| ) -> slist[_K]: ... | |
| def toMap(self: stream[Tuple[_T, _V]]) -> sdict[_T, _V]: ... |
🤖 Prompt for AI Agents
In streamerate/__init__.pyi around lines 148 to 151, the stub for sorted omits
the legacy cmp parameter while the runtime still accepts a cmp callable; restore
the parameter by updating the sorted signature to include cmp:
Optional[Callable[[Any, Any], int]] = None (after key and before reverse or with
appropriate placement), and adjust imports/typing if needed so type checkers
accept calls using cmp without breaking existing annotations.
There was a problem hiding this comment.
I cant find any cmp
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation of the sorted method in the streamerate codebase
# First, find Python source files (not stubs)
fd -e py -E "*.pyi" | head -20
# Search for the sorted method definition
rg -n -A5 "def sorted" --type py -g "!*.pyi"
Length of output: 580
@MorariMaxim You're absolutely right, and I apologize for the confusion.
I've verified the actual implementation in streamerate/streams.py (line 861), and the sorted method only accepts key and reverse parameters:
def sorted(self, key=None, reverse=False):
return slist(sorted(self, key=key, reverse=reverse))There is no cmp parameter in the implementation. My previous suggestion was incorrect—the stub is actually correct as-is. The cmp parameter was removed from Python 3's sorted() built-in, and this library doesn't support it either.
Would you like me to resolve my previous comment since it's not valid?
| def zip(self) -> stream[object]: ... | ||
| def distinct(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... | ||
| def unique(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... |
There was a problem hiding this comment.
Model zip as a true var-args zipper.
The concrete implementation forwards to itertools.zip, so callers can (and do) pass additional iterables. The current stub forbids that and drops tuple typing. Please expose the var-args and tuple result.
Apply this diff:
- def zip(self) -> stream[object]: ...
+ def zip(self, *iterables: Iterable[Any]) -> stream[Tuple[Any, ...]]: ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def zip(self) -> stream[object]: ... | |
| def distinct(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... | |
| def unique(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... | |
| def zip(self, *iterables: Iterable[Any]) -> stream[Tuple[Any, ...]]: ... | |
| def distinct(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... | |
| def unique(self, key: Optional[Callable[[_K], _V]] = None) -> stream[_K]: ... |
🤖 Prompt for AI Agents
In streamerate/__init__.pyi around lines 197–199, the zip stub is currently
non-variadic and returns stream[object]; change it to model zip as a true
var-args zipper that accepts any number of additional iterables and returns a
stream of tuples containing the zipped elements. Replace the current signature
with a variadic-iterables version (e.g. def zip(self, *others: Iterable[Any]) ->
stream[Tuple[Any, ...]]) or, if you want precise typing, use PEP 646
TypeVarTuple and Unpack to express def zip(self, *others: Iterable[Unpack[Ts]])
-> stream[Tuple[_K, Unpack[Ts]]]; ensure required typing imports (TypeVarTuple,
Unpack, Tuple, Iterable, Any) are present and fall back to Tuple[Any, ...] when
PEP 646 is not used.
There was a problem hiding this comment.
you are talking about implementation here, its fixed
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current state of the zip method in the stub file
cd /tmp/repos/asuiu/streamerate
git checkout type_stub
grep -n -A 1 "def zip" streamerate/__init__.pyi || echo "Pattern not found"
Length of output: 278
🏁 Script executed:
#!/bin/bash
# Check the latest commits on the PR branch and the current state of the file
git log --oneline -5 type_stub 2>/dev/null || git log --oneline -5
# Show the exact current content around the zip method
sed -n '195,200p' streamerate/__init__.pyi
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Check the actual implementation file to see what zip does
if [ -f streamerate/__init__.py ]; then
echo "=== Checking actual implementation of zip method ==="
grep -n -A 10 "def zip" streamerate/__init__.py | head -20
else
echo "Implementation file not found"
fi
# Also check if there are any other Python files in streamerate
ls -la streamerate/
Length of output: 408
🏁 Script executed:
#!/bin/bash
# Check the actual zip implementation in streams.py
echo "=== Checking zip method in streams.py ==="
grep -n -B 2 -A 10 "def zip" streamerate/streams.py
# Also check what __init__.py imports/exports
echo ""
echo "=== Checking __init__.py ==="
cat streamerate/__init__.py
Length of output: 853
@MorariMaxim, you're absolutely right, and I apologize for the incorrect suggestion.
I reviewed the actual implementation in streams.py (line 1183):
def zip(self) -> "stream[_V]":
return stream(zip(*(self.toList())))The method indeed takes no parameters - it unpacks the stream's own elements into Python's builtin zip(). Your stub signature correctly reflects this. My earlier suggestion to add *iterables: Iterable[Any] was wrong.
The only minor refinement you might consider is the return type - stream[Tuple[Any, ...]] would be slightly more precise than stream[object], but that's optional and the current signature is functionally correct.
Thank you for the correction! 🐰
/\___/\
( o o )
( =^= ) "You're right,
( ) my mistake!"
( )
( )))))))
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
19-21: Minor oddity in gevent constraint syntax.The gevent dependency uses a matrix syntax with a redundant Python constraint and an unusual upper bound:
gevent = [ {version = ">=24.2.1", python = ">=3.10,<=4.0.0"} ]The
python = ">=3.10,<=4.0.0"is redundant since the project already requirespython >= 3.10. Additionally,<=4.0.0is unusual when the project constraint is<4.0.0. Simplify to:gevent = ">=24.2.1"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (macos-latest, 3.11)
- GitHub Check: build (macos-latest, 3.10)
- GitHub Check: build (windows-latest, 3.11)
🔇 Additional comments (4)
pyproject.toml (4)
16-16: Verify Python 3.10+ minimum is reflected across all tooling and CI.This is a breaking change that bumps the minimum from (presumably) 3.9 to 3.10. Ensure this is:
- Documented in release notes or migration guide
- Reflected in the CI workflow matrix (the AI summary mentions trimming to 3.10–3.13)
- Tested across all target Python versions
Please confirm that CI workflows have been updated to reflect the new Python 3.10–3.13 matrix and that all dependencies in
pyproject.tomlare compatible with Python 3.10+.
25-27: Conditional Python constraints in dev dependencies are acceptable but could be clearer.Both
pandasandpydanticare now conditional on Python >= 3.10. Since these are dev dependencies, this is acceptable given the project's new minimum. However, consider documenting why these specific versions require Python 3.10+ (e.g., if they dropped support for earlier versions).Also applies to: 32-34
8-8: Version consistency note betweentool.poetryandtool.commitizen.Line 8 documents that the poetry version is "kept in sync with
tool.commitizenconfig via automation," but the commitizen version on line 85 is "1.0.5" while the poetry version is "1.2.1". If these should be in sync, verify the automation is working correctly. If they serve different purposes (e.g., commitizen tracks conventional commits schema version rather than package version), clarify the comment on line 8.Also applies to: 85-85
28-28: pyxtension patch bump is safe.The minor version bump from 1.17.0 to 1.17.1 is a patch update and should be backward compatible. ✓
Type stub for type checkers(pyright mostly, since using it on strict mode revealed the type errors)
I tried to make the stub reflect the internal structure as much as possible
Summary by CodeRabbit