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

Refactor: split_mora() の分割 #792

Closed
3 tasks done
tarepan opened this issue Nov 28, 2023 · 2 comments · Fixed by #973
Closed
3 tasks done

Refactor: split_mora() の分割 #792

tarepan opened this issue Nov 28, 2023 · 2 comments · Fixed by #973

Comments

@tarepan
Copy link
Contributor

tarepan commented Nov 28, 2023

内容

要望: split_mora() 関数の二分割によるリファクタリング

モーラを音素に分割する split_mora() 関数は現在、以下の3箇所で利用されている。

_, _, vowel_indexes_data = split_mora(phoneme_data_list)

_, _, vowel_indexes_data = split_mora(phoneme_data_list)

(
consonant_phoneme_data_list,
vowel_phoneme_data_list,
_,
) = split_mora(phoneme_data_list)

このように、2/3のユースケースでは vowel_indexes_data 返り値のみを利用している。

split_mora() 内部では早期に vowel_indexes_data を計算しており、これを取り出すだけなら以後の全演算が不要である。

"""
vowel_indexes = [
i for i, p in enumerate(phoneme_list) if p.phoneme in mora_phoneme_list
]

また split_mora() という関数名から母音インデックス取り出しを想起することは難しい。

上記の理由から母音インデックス取り出し関数の切り出しによるリファクタリングを提案します。

Pros 良くなる点

  • 不要コード実行の排除による実行速度の向上
  • 関数名変更による可読性の向上

Cons 悪くなる点

  • 無し

実現方法

  • _find_vowel_indexes() 関数への機能切り出し

VOICEVOXのバージョン

0.14.10

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux
@github-actions github-actions bot added OS 依存:linux Linux に依存した現象 OS 依存:mac macOS に依存した現象 OS 依存:win Windows に依存した現象 labels Nov 28, 2023
@Hiroshiba
Copy link
Member

提案ありがとうございます! 方針として良いのかなと思いました!!

@tarepan
Copy link
Contributor Author

tarepan commented Nov 29, 2023

👍
#790 に依存しているため、当該PRマージ後に着手します。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants