-
Notifications
You must be signed in to change notification settings - Fork 25
feat: replace mypy with ty #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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. WalkthroughUpdated project dependencies and CI type checking; moved some typing imports to typing_extensions; broadened audio utilities to accept any iterable of byte chunks and adjusted tests and notebook example accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (2)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to replace mypy with ty as the Python type checker. The changes add ty as a development dependency, introduce typing-extensions as a runtime dependency, and make adjustments to type-related code including import statements and type checking patterns.
- Adds ty (v0.0.2) and typing-extensions (>=4.15.0) as dependencies
- Updates isinstance checks from explicit Iterator type checking to negated bytes checking
- Moves Concatenate and ParamSpec imports from typing to typing-extensions
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds ty package (v0.0.2) with platform-specific wheels, typing-extensions dependency, and self-referential dev dependency for fish-audio-sdk[utils] |
| pyproject.toml | Adds typing-extensions to runtime dependencies, ty to dev dependencies, fish-audio-sdk[utils] to dev dependencies, and workspace source configuration |
| src/fishaudio/utils/save.py | Changes isinstance check from isinstance(audio, Iterator) to not isinstance(audio, bytes) |
| src/fishaudio/utils/play.py | Changes isinstance check from isinstance(audio, Iterator) to not isinstance(audio, bytes) and adds ty-specific ignore comment |
| src/fish_audio_sdk/io.py | Moves Concatenate and ParamSpec imports from typing module to typing-extensions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
pyproject.toml(4 hunks)src/fish_audio_sdk/io.py(1 hunks)src/fishaudio/utils/play.py(1 hunks)src/fishaudio/utils/save.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Python
src/fishaudio/utils/play.py
[error] 91-91: PortAudio library not found. Importing sounddevice raised OSError during test_play_sounddevice_mode.
⏰ 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). (1)
- GitHub Check: Agent
🔇 Additional comments (5)
src/fish_audio_sdk/io.py (1)
14-15: LGTM! Fixes Python 3.9 compatibility.The move of
ConcatenateandParamSpecfromtypingtotyping_extensionsis correct. These types were only added to the standardtypingmodule in Python 3.10, but the project supports Python 3.9+. Thetyping_extensionspackage provides backports for earlier Python versions.pyproject.toml (2)
87-88: LGTM! Standard uv workspace configuration.The
[tool.uv.sources]section correctly configures the workspace mode for theuvtool, enabling workspace-based source discovery during development.
14-14: No action required.typing-extensions>=4.15.0includes bothConcatenateandParamSpec— these features have been available since version 4.0.0, so the version constraint is compatible with the imports insrc/fish_audio_sdk/io.py.src/fishaudio/utils/play.py (1)
60-60: LGTM! Appropriate use of type checker ignore comment.The
# ty: ignore[unresolved-import]comment is appropriate for the IPython import, which may not be available in the type-checking environment. This prevents false positives when IPython is an optional dependency.src/fishaudio/utils/save.py (1)
30-31: The behavioral change from isinstance(audio, Iterator) to if not isinstance(audio, bytes) is correct but verify all calling paths.The change makes the code more permissive: it now attempts to join any non-bytes input rather than checking for explicit Iterator instances. This is technically sound from a duck-typing perspective, but the docstring and type annotation (
Union[bytes, Iterator[bytes]]) constrain expected inputs to bytes or iterators.All current callers (tests, examples, docstrings) pass bytes or results from
client.tts.convert(), which should match the type annotation. Ensure future callers never pass unchecked iterables like lists or generators directly, asb"".join()will fail on non-bytes elements.
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.