chore: add ruff linting rules and fix violations#109
Conversation
📝 WalkthroughWalkthroughWide-scope typing and import modernizations across the codebase: replace Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a Ruff lint configuration and applies a broad set of style/type-hint cleanups across the SDK, tests, and examples to satisfy the new lint rules.
Changes:
- Add Ruff lint rule configuration to
pyproject.toml. - Refactor imports and type annotations (e.g.,
typing.List→list,typing.Iterator→collections.abc.Iterator, absolute imports). - Update a few tests/examples to comply with pathlib and simplification rules (e.g.,
tmp_pathusage,yield from,Path(...).stat()).
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_voices.py | Import ordering/type import sorting cleanup. |
| tests/unit/test_utils.py | Switch save() tests to use tmp_path + real filesystem reads; import cleanup. |
| tests/unit/test_types.py | Reordered imported types to satisfy import-sorting. |
| tests/unit/test_tts_realtime.py | Import ordering cleanup. |
| tests/unit/test_tts.py | Import ordering cleanup. |
| tests/unit/test_realtime.py | Import ordering + small simplification in a mock (else removal). |
| tests/unit/test_core.py | Import ordering + combine nested context managers. |
| tests/unit/test_client.py | Import ordering cleanup. |
| tests/unit/test_account.py | Import ordering cleanup. |
| tests/unit/conftest.py | Import ordering cleanup. |
| tests/integration/test_tts_websocket_integration.py | Import ordering + yield from simplification. |
| tests/integration/test_account_integration.py | Import ordering cleanup. |
| tests/integration/conftest.py | Simplify bytes vs chunk-join assignment. |
| src/fishaudio/utils/stream.py | Use collections.abc.Iterator and absolute import for exceptions. |
| src/fishaudio/utils/save.py | Use pathlib.Path.open() (PTH) and modern typing imports. |
| src/fishaudio/utils/play.py | Use collections.abc.Iterable and absolute import for exceptions. |
| src/fishaudio/types/voices.py | Replace typing.List with builtin list generics. |
| src/fishaudio/types/tts.py | Replace typing.List with builtin list generics. |
| src/fishaudio/types/shared.py | Replace typing.List with builtin list generics. |
| src/fishaudio/types/asr.py | Replace typing.List with builtin list generics. |
| src/fishaudio/resources/voices.py | Type hint updates + absolute imports; introduces builtins.list[...] annotations. |
| src/fishaudio/resources/tts.py | Typing/import refactor (collections.abc, absolute imports), minor generator simplification. |
| src/fishaudio/resources/realtime.py | Use builtin dict[...] typing + absolute import for exceptions; minor control-flow simplification. |
| src/fishaudio/resources/asr.py | Absolute import cleanup. |
| src/fishaudio/resources/account.py | Absolute import cleanup. |
| src/fishaudio/core/websocket_options.py | Dict → dict typing update. |
| src/fishaudio/core/request_options.py | Dict → dict typing update. |
| src/fishaudio/core/iterators.py | Use collections.abc iterator types. |
| src/fishaudio/core/client_wrapper.py | Dict → dict typing update + import normalization. |
| src/fishaudio/client.py | Import ordering cleanup. |
| src/fish_audio_sdk/websocket.py | Import ordering + collections.abc iterator types. |
| src/fish_audio_sdk/schemas.py | Add future annotations + move some imports under TYPE_CHECKING. |
| src/fish_audio_sdk/io.py | Add future annotations + typing/import refactor + small return simplifications. |
| src/fish_audio_sdk/apis.py | Add future annotations + move Generator to TYPE_CHECKING. |
| src/fish_audio_sdk/init.py | Reorder exports/imports. |
| scripts/copy_docs.py | Add future annotations import. |
| pyproject.toml | Add Ruff lint configuration and pyupgrade settings. |
| examples/getting_started.ipynb | Reformat cell source to multi-line list form. |
| examples/getting-started/01_simple_tts.py | Replace os.path.getsize with Path(...).stat() for PTH compliance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import builtins | ||
| from typing import Optional, Union | ||
|
|
||
| from ..core import OMIT, AsyncClientWrapper, ClientWrapper, RequestOptions | ||
| from ..types import PaginatedResponse, Visibility, Voice | ||
| from fishaudio.core import OMIT, AsyncClientWrapper, ClientWrapper, RequestOptions | ||
| from fishaudio.types import PaginatedResponse, Visibility, Voice |
There was a problem hiding this comment.
Importing builtins just to write builtins.list[...] in type annotations is unnecessary and inconsistent with the rest of this file (which already uses list[str]). Prefer list[...] directly and drop the builtins import unless there’s a specific shadowing concern to document.
![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
🤖 Fix all issues with AI agents
In `@src/fish_audio_sdk/schemas.py`:
- Around line 1-9: The module currently imports datetime and decimal only under
TYPE_CHECKING which leaves those names absent at runtime and breaks Pydantic
model resolution (with from __future__ import annotations present); move the
imports for datetime and decimal out of the TYPE_CHECKING guard so they are
available at runtime (ensure module-level imports of datetime and decimal are
present) so classes like ModelEntity and APICreditEntity can resolve
datetime.datetime and decimal.Decimal when Pydantic evaluates annotations.
In `@tests/unit/test_core.py`:
- Around line 104-108: The test uses a parenthesized multi-context with
(patch.dict(...), pytest.raises(...)) which is only valid in Python 3.10+;
update the test to use nested with statements so it remains Python 3.9
compatible: open a with patch.dict("os.environ", {}, clear=True): block and
inside it open a with pytest.raises(ValueError, match="API key must be
provided"): block, then call ClientWrapper() within the inner block.
🧹 Nitpick comments (8)
src/fishaudio/utils/save.py (1)
5-5: Consider usingUnion→X | Ysyntax withfrom __future__ import annotations.The broader PR adds
from __future__ import annotationsacross the codebase. This file could follow suit, allowingbytes | Iterable[bytes]instead ofUnion[bytes, Iterable[bytes]]and dropping theUnionimport.♻️ Proposed refactor
+from __future__ import annotations + from collections.abc import Iterable from pathlib import Path -from typing import Union -def save(audio: Union[bytes, Iterable[bytes]], filename: str) -> None: +def save(audio: bytes | Iterable[bytes], filename: str) -> None:src/fishaudio/utils/play.py (2)
5-8: Import modernization looks good, butUnionis still used.The PR modernizes typing imports across the codebase (including
from __future__ import annotationsin other files). Consider addingfrom __future__ import annotationshere too and replacingUnion[bytes, Iterable[bytes]]on line 21 withbytes | Iterable[bytes], which would also let you drop theUnionimport on line 6.♻️ Suggested diff
+from __future__ import annotations + """Audio playback utility.""" import io import subprocess from collections.abc import Iterable -from typing import Union from fishaudio.exceptions import DependencyErrorAnd on line 21:
- audio: Union[bytes, Iterable[bytes]], + audio: bytes | Iterable[bytes],
11-17:_is_installedis duplicated instream.py.Both
play.pyandstream.pydefine an identical_is_installedhelper. Consider extracting it to a shared location (e.g.,fishaudio/utils/__init__.pyor a small_common.pymodule) to reduce duplication.src/fishaudio/utils/stream.py (1)
66-72: Pre-existing: repeatedbytesconcatenation is O(n²).
audio_buffer += chunkcopies the entire buffer on each iteration. For large audio streams this can be slow. Abytearrayorlistof chunks joined at the end would be more efficient. Not introduced by this PR, so just a heads-up.♻️ Suggested improvement
- audio_buffer = b"" + chunks: list[bytes] = [] try: for chunk in audio_stream: if chunk and mpv_process.stdin: mpv_process.stdin.write(chunk) mpv_process.stdin.flush() - audio_buffer += chunk + chunks.append(chunk) finally: # Cleanup if mpv_process.stdin: mpv_process.stdin.close() mpv_process.wait() - return audio_buffer + return b"".join(chunks)src/fishaudio/resources/tts.py (1)
11-32: Inconsistent import style: absolute imports for cross-package, relative for same-package.Lines 11–17 use absolute imports (
from fishaudio.core ...,from fishaudio.types ...), but line 32 still uses a relative import (from .realtime ...). This is a common and defensible pattern (absolute for cross-package, relative for intra-package), but if the intent of this PR is to standardize on absolute imports, consider making it consistent.Optional: switch to absolute import
-from .realtime import aiter_websocket_audio, iter_websocket_audio +from fishaudio.resources.realtime import aiter_websocket_audio, iter_websocket_audiotests/unit/test_tts.py (1)
403-421: Pre-existing:list()wrappingconvert()return value is misleading.
tts_client.convert(...)returnsbytes. Wrapping it inlist()produces a list of integers (byte values), which is likely unintentional. The test still passes because it only inspects the mock'scall_args, but thelist()call is confusing. Not introduced by this PR, so no action needed now.Optional cleanup
- list( - tts_client.convert(text="Hello", format="wav", speed=1.3, latency="normal") - ) + tts_client.convert(text="Hello", format="wav", speed=1.3, latency="normal")src/fishaudio/core/request_options.py (1)
3-25: Ensure Python 3.9+ is the minimum supported version.
dict[str, str]as a runtime type annotation (withoutfrom __future__ import annotations) requires Python 3.9+. This is consistent with the broader modernization in this PR, but worth confirming. Also, consider addingfrom __future__ import annotationshere for consistency with other files in the PR (e.g.,src/fish_audio_sdk/apis.py), which would also allow replacingOptional[...]with... | None.src/fishaudio/core/client_wrapper.py (1)
63-76: Runtimedict[str, str]withoutfrom __future__ import annotations.Same note as
request_options.py—dict[str, str]anddict[str, Any]as runtime annotations require Python 3.9+. Consider addingfrom __future__ import annotationsfor consistency withsrc/fish_audio_sdk/apis.py, or confirm the project targets 3.9+.
1 participant
Summary by CodeRabbit
New Features
Chores
Tests