Skip to content
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

整理: グローバル特徴量適用の関数化 #819

Merged
merged 8 commits into from
Dec 9, 2023

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Dec 6, 2023

内容

グローバル特徴量適用の関数化によるリファクタリング

AudioQuery を介してユーザーから与えられるグローバル特徴量は以下の7系統:

  • speedScale
  • pitchScale
  • intonationScale
  • volumeScale
  • (silence)
    • prePhonemeLength
    • postPhonemeLength
  • outputSamplingRate
  • outputStereo

それぞれが独立したユーザー機能であるため、これらの適用を関数として独立しテストを追加する。

関連 Issue

step1a of #815

@tarepan tarepan requested a review from a team as a code owner December 6, 2023 03:38
@tarepan tarepan requested review from y-chan and removed request for a team December 6, 2023 03:38
Copy link

github-actions bot commented Dec 6, 2023

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 481 327 coverage-32%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 19 1 coverage-95%
voicevox_engine/cancellable_engine.py 91 71 coverage-22%
voicevox_engine/dev/core/init.py 2 0 coverage-100%
voicevox_engine/dev/core/mock.py 27 12 coverage-56%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 38 2 coverage-95%
voicevox_engine/engine_manifest/EngineManifest.py 34 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 12 0 coverage-100%
voicevox_engine/engine_manifest/init.py 3 0 coverage-100%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/library_manager.py 93 5 coverage-95%
voicevox_engine/metas/Metas.py 33 0 coverage-100%
voicevox_engine/metas/MetasStore.py 29 12 coverage-59%
voicevox_engine/metas/init.py 2 0 coverage-100%
voicevox_engine/model.py 162 9 coverage-94%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/morphing.py 70 46 coverage-34%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 81 2 coverage-98%
voicevox_engine/preset/init.py 4 0 coverage-100%
voicevox_engine/setting/Setting.py 11 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 19 0 coverage-100%
voicevox_engine/setting/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/core_wrapper.py 202 147 coverage-27%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 59 30 coverage-49%
voicevox_engine/synthesis_engine/synthesis_engine.py 166 13 coverage-92%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 71 10 coverage-86%
voicevox_engine/user_dict.py 144 12 coverage-92%
voicevox_engine/utility/init.py 5 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 10 0 coverage-100%
voicevox_engine/utility/path_utility.py 26 8 coverage-69%
TOTAL 2221 712 coverage-68%

@tarepan
Copy link
Contributor Author

tarepan commented Dec 6, 2023

2023-12-06T19:08+09:00
apply_speed() を追加しました。

@tarepan
Copy link
Contributor Author

tarepan commented Dec 6, 2023

2023-12-06T19:12+09:00
pad_with_silence()apply_silence() に名称統一しました。

追加終了、Review可能です。

@tarepan
Copy link
Contributor Author

tarepan commented Dec 6, 2023

2023-12-07T00:35+09:00
apply_sampling_rate()apply_stereo() を追加しました。

追加終了、Review可能です。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ほぼLGTMです!!
関数名とコメントに関してだけのコメントです。

ちょっとコメントに関してコメントをば。(?)
コメントの頻度は良い感じかなと思いました!
ただちょっと正確性が気になるので、そこ意識できればとても良くなるなと感じました!
偉そうなことを言ってしまうのですが、何かの助けになればと思ってお伝えします 🙇

  • オリジナルワードはもう少し避けるのを意識すると良いかも
    • 正確に伝わらないため
    • もちろん頻出するものは別、ドメイン用語にしましょう!
  • 記号は避けると良いかも
    • 複数の意味を持つため(例えば=は代入?イコール?)
    • もちろんコードの例示は別
  • コードの読み手がほしい情報を書くと良いかも
    • 今回だとApply: Convert: は情報量が増えてなくて、書くことが目的になってるかも
    • 要約はありだと思います!

コメントの目的は「可読性の向上」と「コードから読めない意図の伝達」だと思います!
書くのは良いことなので、あとは意識だけかなと思いました!!

参考
「リーダブルコード」を読む (第5章「コメントすべきことを知る」第6章「コメントは正確で簡潔に」)

voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
@tarepan
Copy link
Contributor Author

tarepan commented Dec 9, 2023

@Hiroshiba
全指摘箇所の反映・テストパスを確認しました。Re-reviewよろしくお願いします。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!!

@Hiroshiba Hiroshiba merged commit 154d5d0 into VOICEVOX:master Dec 9, 2023
3 checks passed
@tarepan tarepan deleted the refactor/apply branch December 10, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants