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

整理: .mutex 移植 #813

Closed
wants to merge 1 commit into from
Closed

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Dec 5, 2023

内容

.mutexSynthesisEngineCoreWrapper 移植によるリファクタリング

関連 Issue

resolve #809

@tarepan tarepan requested a review from a team as a code owner December 5, 2023 04:46
@tarepan tarepan requested review from y-chan and removed request for a team December 5, 2023 04:46
Copy link

github-actions bot commented Dec 5, 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 208 152 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 2227 717 coverage-68%

@Hiroshiba Hiroshiba requested review from Hiroshiba and removed request for y-chan December 12, 2023 16:02
@Hiroshiba
Copy link
Member

こちらのプルリクエストに関して、↓で議論した内容と若干齟齬が生じる形になってることに気づきました 🙇

「CoreWrapper の責務範囲は "コアの関数を Python から実行可能にする" である」

やり方は2通りありそうで、1つはこの移植をやめることと、もう1つはCoreWrapperの責務内にmutexも含めるかかなと。
すごい微妙なとこで、ぶっちゃけどっちでもありな気がするのですが、CoreWrapperの役割をきれいにするのであれば前者が、voicevix_engineパッケージを綺麗にするのであれば後者が良さそうかなと。

・・・・・・・・・・うーーーーーん、どっちが良いかすごく迷っています。あまりちゃんと理由はつけられないけど、どっちかというとCoreWrapperの範囲が明確な方が進めやすい・・・かも・・・?

@tarepan
Copy link
Contributor Author

tarepan commented Dec 12, 2023

CoreWrapper の範囲が明確な方が進めやすい

👍
PR取り下げます。

議論ログ

2023-12-04: Tarepan 移植提案 issue #809 open
2023-12-05: Hiroshiba 移植Go判断 at issue #809
2023-12-05: Tarepan 本PR #813 open
2023-12-07: Tarepan CoreWrapper 責務明確化 issue #825 open
2023-12-07: Hiroshiba 責務範囲限局の方針明示 at issue #825
2023-12-13: Hiroshiba 本PR review、方針明示(変更)に伴う移植NoGo判断
2023-12-13: Tarepan 本PR review reply、同意につきPR close

@tarepan tarepan closed this Dec 12, 2023
@tarepan tarepan deleted the refactor/mutex branch December 31, 2023 06:49
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.

Refactor: .mutexSynthesisEngineCoreWrapper 移植
2 participants