refactor: class-based design #15

NULL204 · 2024-11-28T09:10:11Z

Summary by Sourcery

Refactor the subtitle translation functionality into a class-based design using the new SubtitleTranslator class. Update the README to reflect these changes and add tests for the new class. Adjust the CI workflow to reorder OS versions in the testing matrix.

Enhancements:

Refactor the code to use a class-based design by introducing the SubtitleTranslator class, encapsulating subtitle translation logic.

CI:

Update CI workflow to adjust the order of OS versions in the matrix.

Documentation:

Update README to reflect the new class-based design and usage of the SubtitleTranslator class.

Tests:

Add new tests for the SubtitleTranslator class to verify subtitle translation from both subtitle files and audio inputs.

modified: yuisub/__main__.py new file: yuisub/sub_translator.py

sourcery-ai · 2024-11-28T09:10:16Z

Reviewer's Guide by Sourcery

This PR refactors the codebase to implement a class-based design by introducing the SubtitleTranslator class. The class encapsulates all subtitle translation functionality, including audio transcription and subtitle file handling. The changes simplify the API surface and improve code organization by moving the core functionality into a single cohesive class.

Class diagram for WhisperModel

classDiagram
    class WhisperModel {
        - name: str
        - device: Optional[Union[str, torch.device]]
        - download_root: Optional[str]
        - in_memory: bool
        + WhisperModel(name, device, download_root, in_memory)
        + transcribe(audio)
    }

File-Level Changes

Change	Details	Files
Introduce new SubtitleTranslator class to encapsulate subtitle translation functionality	Create class constructor with configuration parameters for LLM, Bangumi, and Whisper settings Implement get_subtitles method to handle both audio and subtitle file inputs Add automatic device selection logic for Whisper model initialization Consolidate translation and bilingual subtitle generation into a single workflow	`yuisub/translator.py`
Refactor main script to use the new class-based approach	Replace direct function calls with SubtitleTranslator class usage Simplify command-line argument handling Add input validation to ensure either audio or subtitle file is provided Update error messages and argument descriptions	`yuisub/__main__.py`
Update documentation and examples	Add class-based usage examples to README Simplify code examples by showing the new unified API Update documentation to reflect new class-based architecture	`README.md`
Update test suite for new class-based implementation	Add new test module for SubtitleTranslator class Update existing tests to use new Bangumi token parameter Add CI skip conditions for specific tests Fix import statements and test utilities	`tests/test_translator.py` `tests/test_bangumi.py` `tests/test_sub.py` `tests/test_llm.py` `tests/util.py`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time. You can also use
this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @NULL204 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 3 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

yuisub/sub_translator.py

README.md

modified: yuisub/__main__.py renamed: yuisub/sub_translator.py -> yuisub/translator.py

NULL204 · 2024-11-28T13:23:59Z

@sourcery-ai review

sourcery-ai

Hey @NULL204 - I've reviewed your changes - here's some feedback:

Overall Comments:

There appears to be a bug in get_subtitles() where it uses self.sub_zh instead of the local sub_zh variable in the bilingual() call. This will cause issues since self.sub_zh is never set.

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

yuisub/translator.py

modified: yuisub/__main__.py modified: yuisub/translator.py

codecov · 2024-11-30T06:48:00Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

modified: tests/test_bangumi.py modified: tests/test_llm.py modified: tests/test_sub.py new file: tests/test_translator.py modified: tests/util.py modified: yuisub/translator.py

update

Tohrusky · 2024-12-01T08:52:16Z

@sourcery-ai review

sourcery-ai

Hey @NULL204 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟡 Testing: 3 issues found
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2024-12-01T08:55:52Z

yuisub/translator.py

+        sub: Optional[Union[str, Path, pysubs2.SSAFile]] = None,
+        audio: Optional[Union[str, Any]] = None,
+        styles: Optional[Dict[str, pysubs2.SSAStyle]] = None,
+        ad: Optional[pysubs2.SSAEvent] = advertisement(),  # noqa: B008


issue (bug_risk): Avoid mutable default arguments in method parameters

Mutable default arguments can cause unexpected behavior. Consider using None as the default and creating the advertisement in the method body if needed.

sourcery-ai · 2024-12-01T08:55:52Z

yuisub/sub.py

@@ -112,8 +112,8 @@ async def translate(
        base_url=base_url,
        bangumi_info=bangumi_info,
    )
+    print(summarizer.system_prompt)


suggestion: Remove or replace debug print statement

Consider using a proper logging system instead of print statements if this information is important for debugging.

import logging logging.debug(summarizer.system_prompt)

sourcery-ai · 2024-12-01T08:55:52Z

tests/test_translator.py

+async def test_translator_sub() -> None:
+    translator = SubtitleTranslator(
+        model=util.OPENAI_MODEL,
+        api_key=util.OPENAI_API_KEY,
+        base_url=util.OPENAI_BASE_URL,
+        bangumi_url=util.BANGUMI_URL,
+        bangumi_access_token=util.BANGUMI_ACCESS_TOKEN,
+    )
+
+    sub_zh, sub_bilingual = await translator.get_subtitles(sub=str(util.TEST_ENG_SRT))


suggestion (testing): Test should verify the content of the translated subtitles

The test only checks if the files are saved but doesn't verify the actual content of the translations. Consider adding assertions to check the translated text, timing, and format of both sub_zh and sub_bilingual.

async def test_translator_sub() -> None: translator = SubtitleTranslator( model=util.OPENAI_MODEL, api_key=util.OPENAI_API_KEY, base_url=util.OPENAI_BASE_URL, bangumi_url=util.BANGUMI_URL, bangumi_access_token=util.BANGUMI_ACCESS_TOKEN, ) sub_zh, sub_bilingual = await translator.get_subtitles(sub=str(util.TEST_ENG_SRT)) assert "你好" in str(sub_zh) assert "Hello" in str(sub_bilingual) and "你好" in str(sub_bilingual)

sourcery-ai · 2024-12-01T08:55:52Z

tests/test_translator.py

+@pytest.mark.skipif(os.environ.get("GITHUB_ACTIONS") == "true", reason="Skipping test when running on CI")
+async def test_translator_sub() -> None:
+    translator = SubtitleTranslator(
+        model=util.OPENAI_MODEL,
+        api_key=util.OPENAI_API_KEY,
+        base_url=util.OPENAI_BASE_URL,
+        bangumi_url=util.BANGUMI_URL,
+        bangumi_access_token=util.BANGUMI_ACCESS_TOKEN,
+    )
+


suggestion (testing): Consider using mocks for CI environment instead of skipping tests

Rather than skipping these tests in CI, consider mocking the external dependencies (Whisper model, OpenAI API) to allow these tests to run in all environments. This would provide better test coverage and catch potential issues earlier.

@pytest.mark.asyncio @mock.patch('your_module.SubtitleTranslator.get_subtitles') async def test_translator_sub(mock_get_subtitles) -> None: mock_get_subtitles.return_value = (Mock(), Mock()) translator = SubtitleTranslator( model=util.OPENAI_MODEL, api_key="mock_key", base_url="mock_url", bangumi_url="mock_url", bangumi_access_token="mock_token" ) await translator.get_subtitles(sub=str(util.TEST_ENG_SRT))

sourcery-ai · 2024-12-01T08:55:52Z

tests/test_translator.py

+async def test_translator_audio() -> None:
+    translator = SubtitleTranslator(
+        torch_device=util.DEVICE,
+        whisper_model=util.MODEL_NAME,
+        model=util.OPENAI_MODEL,
+        api_key=util.OPENAI_API_KEY,
+        base_url=util.OPENAI_BASE_URL,
+        bangumi_url=util.BANGUMI_URL,
+        bangumi_access_token=util.BANGUMI_ACCESS_TOKEN,
+    )


suggestion (testing): Add error case tests for the SubtitleTranslator

The tests only cover the happy path. Consider adding tests for error cases such as invalid audio files, network errors, invalid API keys, and other edge cases that could occur during translation.

async def test_translator_audio() -> None: translator = SubtitleTranslator( torch_device=util.DEVICE, whisper_model=util.MODEL_NAME, model=util.OPENAI_MODEL, api_key=util.OPENAI_API_KEY, base_url=util.OPENAI_BASE_URL, bangumi_url=util.BANGUMI_URL, bangumi_access_token=util.BANGUMI_ACCESS_TOKEN, ) sub_zh, sub_bilingual = await translator.get_subtitles(audio=str(util.TEST_AUDIO)) sub_zh.save(util.projectPATH / "assets" / "test.zh.translator.audio.ass") sub_bilingual.save(util.projectPATH / "assets" / "test.bilingual.translator.audio.ass") with pytest.raises(FileNotFoundError): await translator.get_subtitles(audio="nonexistent_file.mp3") with pytest.raises(Exception): invalid_translator = SubtitleTranslator( torch_device=util.DEVICE, whisper_model=util.MODEL_NAME, model=util.OPENAI_MODEL, api_key="invalid_key", base_url=util.OPENAI_BASE_URL, bangumi_url=util.BANGUMI_URL, bangumi_access_token=util.BANGUMI_ACCESS_TOKEN, ) await invalid_translator.get_subtitles(audio=str(util.TEST_AUDIO))

NULL204 added 2 commits November 28, 2024 17:02

modified: README.md

d7c126e

modified: yuisub/__main__.py new file: yuisub/sub_translator.py

Update README.md

e1c1b4e

sourcery-ai bot reviewed Nov 28, 2024

View reviewed changes

yuisub/sub_translator.py Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

NULL204 added 4 commits November 28, 2024 20:10

modified: README.md

e3dd600

modified: yuisub/__main__.py renamed: yuisub/sub_translator.py -> yuisub/translator.py

Merge branch 'main' of https://github.com/NULL204/yuisub

109cd02

Update README.md

bc7aab4

Update README.md

26fe9cf

sourcery-ai bot reviewed Nov 28, 2024

View reviewed changes

yuisub/translator.py Outdated Show resolved Hide resolved

yuisub/translator.py Outdated Show resolved Hide resolved

NULL204 added 4 commits November 28, 2024 21:38

modified: yuisub/translator.py

e6be40e

Merge branch 'main' of https://github.com/NULL204/yuisub

3be24a6

modified: README.md

d495f4f

modified: yuisub/__main__.py modified: yuisub/translator.py

modified: README.md

5cc45c2

NULL204 and others added 10 commits November 30, 2024 16:12

modified: README.md

d61d287

modified: tests/test_bangumi.py modified: tests/test_llm.py modified: tests/test_sub.py new file: tests/test_translator.py modified: tests/util.py modified: yuisub/translator.py

update

cdbb239

update

d08be72

update

e01ca44

update

4f645c7

update

87fda3a

update

ff2aed2

update

c74e42f

update

252c99b

Merge pull request #2 from TohruskyDev/main

235a148

update

sourcery-ai bot reviewed Dec 1, 2024

View reviewed changes

Tohrusky changed the title ~~Refactor this code into a class-based design using classes and object-oriented principles.~~ refactor: class-based design Dec 1, 2024

Tohrusky merged commit f991615 into TensoRaws:main Dec 1, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: class-based design #15

refactor: class-based design #15

NULL204 commented Nov 28, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Nov 28, 2024 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

NULL204 commented Nov 28, 2024

sourcery-ai bot left a comment

codecov bot commented Nov 30, 2024

Tohrusky commented Dec 1, 2024

sourcery-ai bot left a comment

sourcery-ai bot Dec 1, 2024

sourcery-ai bot Dec 1, 2024

sourcery-ai bot Dec 1, 2024

sourcery-ai bot Dec 1, 2024

sourcery-ai bot Dec 1, 2024

refactor: class-based design #15

refactor: class-based design #15

Conversation

NULL204 commented Nov 28, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Nov 28, 2024 • edited Loading

Reviewer's Guide by Sourcery

Class diagram for WhisperModel

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Choose a reason for hiding this comment

NULL204 commented Nov 28, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 30, 2024

Welcome to Codecov 🎉

Tohrusky commented Dec 1, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Dec 1, 2024

Choose a reason for hiding this comment

sourcery-ai bot Dec 1, 2024

Choose a reason for hiding this comment

sourcery-ai bot Dec 1, 2024

Choose a reason for hiding this comment

sourcery-ai bot Dec 1, 2024

Choose a reason for hiding this comment

sourcery-ai bot Dec 1, 2024

Choose a reason for hiding this comment

NULL204 commented Nov 28, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Nov 28, 2024 •

edited

Loading