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

整理: 辞書クラス・関数をモジュール間で移動 #1329

Merged
merged 19 commits into from
Jun 2, 2024

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented May 26, 2024

内容

概要: 辞書クラス・関数をモジュール間で移動してリファクタリング

現在のユーザー辞書は実装が model / part_of_speech_data / user_dict モジュール間で行き来しており、見通しが悪い。
現状調査の結果、以下の 3 つに分類できることがわかった:

  • モデル: API と辞書実装で共有している model
  • ワード: 辞書を構成する個別の言葉(単語)クラスとそのハンドラ
  • マネージャ: 単語の総体を辞書オブジェクトとして管理するクラスとその関連

実装サイズが大きいため、これに従ってモジュール分割すると見通しが改善する。

このような背景から、辞書クラス・関数をモジュール間で移動するリファクタリングを提案します。

#1242 でのテスト有効化後の方がより安全 追記: 済

関連 Issue

無し

@tarepan tarepan requested a review from a team as a code owner May 26, 2024 05:26
@tarepan tarepan requested review from Hiroshiba and removed request for a team May 26, 2024 05:26
@tarepan tarepan marked this pull request as draft May 28, 2024 10:46
@tarepan tarepan changed the title 整理: 言葉関係を part_of_speech_data へ集約 整理: 辞書モデルを機能別 model.py へ集約 May 28, 2024
@tarepan tarepan changed the title 整理: 辞書モデルを機能別 model.py へ集約 整理: 辞書クラス・関数をモジュール間で移動 May 28, 2024
@tarepan tarepan marked this pull request as ready for review May 28, 2024 11:12
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!!

voicevox_engine/user_dict/user_dict_word.py Show resolved Hide resolved
Comment on lines +17 to +164
part_of_speech_detail_2="*",
part_of_speech_detail_3="*",
context_id=1345,
cost_candidates=[
-4445,
49,
1473,
2897,
4321,
5746,
6554,
7362,
8170,
8979,
15001,
],
accent_associative_rules=[
"*",
"C1",
"C2",
"C3",
"C4",
"C5",
],
),
WordTypes.VERB: PartOfSpeechDetail(
part_of_speech="動詞",
part_of_speech_detail_1="自立",
part_of_speech_detail_2="*",
part_of_speech_detail_3="*",
context_id=642,
cost_candidates=[
3100,
6160,
6360,
6561,
6761,
6962,
7414,
7866,
8318,
8771,
13433,
],
accent_associative_rules=[
"*",
],
),
WordTypes.ADJECTIVE: PartOfSpeechDetail(
part_of_speech="形容詞",
part_of_speech_detail_1="自立",
part_of_speech_detail_2="*",
part_of_speech_detail_3="*",
context_id=20,
cost_candidates=[
1527,
3266,
3561,
3857,
4153,
4449,
5149,
5849,
6549,
7250,
10001,
],
accent_associative_rules=[
"*",
],
),
WordTypes.SUFFIX: PartOfSpeechDetail(
part_of_speech="名詞",
part_of_speech_detail_1="接尾",
part_of_speech_detail_2="一般",
part_of_speech_detail_3="*",
context_id=1358,
cost_candidates=[
4399,
5373,
6041,
6710,
7378,
8047,
9440,
10834,
12228,
13622,
15847,
],
accent_associative_rules=[
"*",
"C1",
"C2",
"C3",
"C4",
"C5",
],
),
}
Copy link
Member

Choose a reason for hiding this comment

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

これがここに来たのは逆にごっちゃになっちゃになるかもです。

これと、あとpriority周りは分けちゃっても良いかも。や〜一緒でも問題ないかも。。

@Hiroshiba Hiroshiba merged commit 4b11c23 into VOICEVOX:master Jun 2, 2024
4 checks passed
@tarepan tarepan deleted the refactor/word_module branch June 2, 2024 06:56
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