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

TST: user_dict_startup_processingのテストを追加。 #483

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

sabonerune
Copy link
Contributor

内容

user_dict_startup_processing()のテストを追加しました。

その他

user_dict_startup_processing()update_dict()とほぼ重複している上にユーザー辞書がある場合2回コンパイルが行われています。
削除するべきだと思ったのですが本当にそれでいいのか確認するためにまずはテストから作成しました。

テストのためuser_dict内の関数に引数を追加
@github-actions
Copy link

github-actions bot commented Oct 5, 2022

Coverage Result

Resultを開く
Name Stmts Miss Cover
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 75 0 coverage-100%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 36 2 coverage-94%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/model.py 154 7 coverage-95%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetLoader.py 34 1 coverage-97%
voicevox_engine/preset/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/core_wrapper.py 206 166 coverage-19%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 57 49 coverage-14%
voicevox_engine/synthesis_engine/synthesis_engine.py 133 12 coverage-91%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 67 9 coverage-87%
voicevox_engine/user_dict.py 134 6 coverage-96%
voicevox_engine/utility/init.py 3 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 35 3 coverage-91%
voicevox_engine/utility/path_utility.py 26 6 coverage-77%
TOTAL 1240 265 coverage-79%

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!!

丁寧な進行ありがとうございます!!
@takana-v さんのレビューも頂けると心強いです…!

@Hiroshiba Hiroshiba requested a review from takana-v October 5, 2022 21:52
@takana-v
Copy link
Member

takana-v commented Oct 6, 2022

LGTM!!

user_dict_startup_processing()がupdate_dict()とほぼ重複している上にユーザー辞書がある場合2回コンパイルが行われています。

元々user_dict_startup_processing()は、コンパイルされた辞書が無い場合にデフォルト辞書を用いてコンパイル済み辞書を作り、適用するという関数でした。
その後、#421VOICEVOX/voicevox#936 の問題が発生し、現在の形になっています。

user_dict_startup_processing()で行いたい事は3つあります。

  • コンパイルされた辞書が無い場合はデフォルト辞書をコンパイルする
  • ユーザー辞書がある場合は、デフォルト辞書と結合してコンパイルする
    • default.csvの変更を反映させるためです
  • コンパイルした辞書を反映させる

これらはupdate_dict()でも行うことができるので、user_dict_startup_processing()の削除は可能だと思います。

@takana-v takana-v merged commit 7421f7b into VOICEVOX:master Oct 6, 2022
@sabonerune
Copy link
Contributor Author

ありがとうございます。
user_dict_startup_processingの削除をしてみようと思います。

@sabonerune sabonerune deleted the test-user-dict-init branch October 6, 2022 22:15
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.

3 participants