Conversation
…d singleton method
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds Pyright to dev tooling and CI, restructures dev dependency/type-checker settings, broadens EncodeOptions.filter and encode logic to accept generic Sequences (excluding str/bytes/bytearray), tweaks Undefined.new typing, and adds tests for sequence-filter and decoder-signature fallback behaviors. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR integrates Pyright as a secondary static type checker to complement mypy, strengthens type annotations for better cross-checker compatibility, and refines version-specific dependency constraints for mypy. The changes ensure more robust type checking across different Python versions and implementations (CPython, PyPy).
Changes:
- Added Pyright to the development tooling and CI linting workflow
- Updated type annotations from
ListtoSequencefor filter parameters to improve type accuracy - Refined mypy dependency constraints with version-specific conditions for Python <3.9 and PyPy
- Added conditional logic in
_encodeto handle non-integer keys when accessing sequence objects
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Adds pyright testenv and integrates it into the linters workflow |
| pyproject.toml | Adds pyright to dev dependencies with version-specific mypy constraints and Pyright configuration |
| requirements_dev.txt | Updates mypy dependency constraints with Python version conditions and adds pyright |
| .github/copilot-instructions.md | Updates linters documentation to include pyright in the toolchain list |
| src/qs_codec/models/encode_options.py | Changes filter type annotation from List to Sequence |
| src/qs_codec/encode.py | Changes filter parameter type and adds conditional handling for non-integer keys in sequences |
| src/qs_codec/models/undefined.py | Simplifies __new__ method signature by removing explicit type hints |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1261 1267 +6
=========================================
+ Hits 1261 1267 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/qs_codec/models/encode_options.py (1)
70-75:⚠️ Potential issue | 🟡 MinorUpdate the filter docstring to mention sequences.
The field now accepts sequences, but the docstring still says “list.” Please update for accuracy.
Suggested change
- - If a list is provided, only those keys/indices are retained. + - If a sequence is provided, only those keys/indices are retained.As per coding guidelines: Keep docstrings descriptive and parity-focused; match the existing module docstring style.
src/qs_codec/encode.py (1)
73-80:⚠️ Potential issue | 🟠 MajorFilter now accepts
Sequence, but runtime still only honors list/tuple.With the updated type, passing a non-list/tuple sequence (e.g.,
range) will be silently ignored. Please broaden the runtime checks to match the new contract, while excluding strings/bytes.Suggested fix
@@ -import typing as t +import typing as t +from collections.abc import Sequence @@ - elif isinstance(options.filter, (list, tuple)): - obj_keys = list(options.filter) + elif isinstance(options.filter, Sequence) and not isinstance(options.filter, (str, bytes, bytearray)): + obj_keys = list(options.filter) @@ - elif isinstance(filter, (list, tuple)): + elif isinstance(filter, Sequence) and not isinstance(filter, (str, bytes, bytearray)): # Iterable filter restricts traversal to a fixed key/index set. obj_keys = list(filter)Also applies to: 170-170, 318-321
…' for key inclusion in EncodeOptions
…handle mapping get exceptions
This pull request adds Pyright static type checking to the development workflow, updates type annotations for better compatibility, and improves handling of sequence types in the codebase. It also ensures that type checking works correctly across different Python versions and implementations.
Type checking and development workflow improvements:
pyrightas a development dependency and integrated it into the lint/type check workflow intox,requirements_dev.txt, and documentation. [1] [2] [3] [4] [5] [6][tool.pyright]configuration section inpyproject.tomlto specify Python version, included source directories, and excluded test and build directories.Type annotation and compatibility fixes:
filterparameter in bothEncodeOptionsand_encodeto useSequenceinstead ofListfor broader compatibility and accuracy. [1] [2]_encodeto avoid errors when non-integer keys are used with sequences.__new__method signature in theUndefinedsingleton for better type compatibility.Python version and implementation-specific dependency management:
mypydependency constraints in bothpyproject.tomlandrequirements_dev.txtto ensure compatibility with different Python versions and PyPy. [1] [2]Summary by CodeRabbit
Chores
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.