整理: docstring追加

[tarepan]

内容

音声合成周りのdocstringの追加

関連 Issue

part of #59

[github-actions]

Coverage Result

Resultを開く

Name	Stmts	Miss	Cover
run.py	481	327
voicevox_engine/init.py	1	0
voicevox_engine/acoustic_feature_extractor.py	19	1
voicevox_engine/cancellable_engine.py	91	71
voicevox_engine/dev/core/init.py	2	0
voicevox_engine/dev/core/mock.py	27	12
voicevox_engine/dev/synthesis_engine/init.py	2	0
voicevox_engine/dev/synthesis_engine/mock.py	38	2
voicevox_engine/engine_manifest/EngineManifest.py	34	0
voicevox_engine/engine_manifest/EngineManifestLoader.py	12	0
voicevox_engine/engine_manifest/init.py	3	0
voicevox_engine/full_context_label.py	162	3
voicevox_engine/kana_parser.py	86	1
voicevox_engine/library_manager.py	93	5
voicevox_engine/metas/Metas.py	33	0
voicevox_engine/metas/MetasStore.py	29	12
voicevox_engine/metas/init.py	2	0
voicevox_engine/model.py	162	9
voicevox_engine/mora_list.py	4	0
voicevox_engine/morphing.py	70	46
voicevox_engine/part_of_speech_data.py	5	0
voicevox_engine/preset/Preset.py	12	0
voicevox_engine/preset/PresetError.py	2	0
voicevox_engine/preset/PresetManager.py	81	2
voicevox_engine/preset/init.py	4	0
voicevox_engine/setting/Setting.py	11	0
voicevox_engine/setting/SettingLoader.py	19	0
voicevox_engine/setting/init.py	3	0
voicevox_engine/synthesis_engine/init.py	5	0
voicevox_engine/synthesis_engine/core_wrapper.py	202	147
voicevox_engine/synthesis_engine/make_synthesis_engines.py	59	30
voicevox_engine/synthesis_engine/synthesis_engine.py	139	13
voicevox_engine/synthesis_engine/synthesis_engine_base.py	71	10
voicevox_engine/user_dict.py	144	12
voicevox_engine/utility/init.py	5	0
voicevox_engine/utility/connect_base64_waves.py	37	0
voicevox_engine/utility/core_version_utility.py	8	1
voicevox_engine/utility/mutex_utility.py	10	0
voicevox_engine/utility/path_utility.py	26	8
TOTAL	2194	712

[Hiroshiba]

ちょっといろいろコメントしてしまったのですが、だいぶ多いのとあまり大事じゃないとこな気がするのでこっちで変更させていただこうと思います！

結構独自の言い回しをされていて、意味が分かる人には伝わるけど、分からない人が見ると誤解されそうだなと感じました。
もうちょっと正確さに倒すと良い塩梅になるかなと･･･！

[Hiroshiba]

表記が特殊だな～と感じました。
コメントを素直に受け取ると「expectとする値はspeedScale適用」で、言いたいことは「expectとする値にspeedScale適用」だと思うので、コメントが不正確かもです。

[Hiroshiba]

Suggested change

	# Rule3: "カナの手前に`_`を入れるとそのカナは無声化される"
	# 無声化

[Hiroshiba]

Suggested change

	# Rule5: "アクセント句末に`？`(全角)を入れることにより疑問文の発音ができる"
	# `？`で疑問文

[Hiroshiba]

LGTM！！！

コメント付与ありがとうございます！！
プルリクエストを頂いて、コメントがないと読みづらいところが多々あることに改めて気づきました。

コメントに関するコメントを色々書きましたが、良い感じの場合は見つかればお互い得かなと思いました！
（ちなみにgithubは、閉じられたコメントはaltを押しながらクリックすると全部展開できます）

[tarepan]

Reviewありがとうございました！

Rule N: "xxxx" のコメントは以下のAquesTalk風記法定義（in README.md）を一切改変せずに貼ったものです：

voicevox_engine/README.md

Lines 59 to 66 in b997385

	記法は次のルールに従います。
	- 全てのカナはカタカナで記述される
	- アクセント句は`/`または`、`で区切る。`、`で区切った場合に限り無音区間が挿入される。
	- カナの手前に`_`を入れるとそのカナは無声化される
	- アクセント位置を`'`で指定する。全てのアクセント句にはアクセント位置を 1 つ指定する必要がある。
	- アクセント句末に`？`(全角)を入れることにより疑問文の発音ができる

どの記法ルールがどの実装に対応しているか初見では理解できなかったため、ルールとの対応づけをコメントに示す意図でした。

suggest 頂いたように処理内容を書くことで可読性が向上することには賛成です。
同時に、「規則がまずありそれを実装する」タイプのコード（例: parser）だと規則-実装対応づけに関するコメントは可読性に資すると感じます。
なんらかの方法でこの対応付けコメントは必要だと思うのですが、コメント形式のいいアイデアが何かあるでしょうか？

[tarepan]

（その2）

あ、推定や推論はエンジンの責務じゃないですね！
comment

関数docstringは「呼び出し側から見た関数の機能」を記述するのが一般的かと思います。
この観点からすると、関数呼び出し側にとって replace_mora_pitch() は「アクセント句系列を渡したらその中に収めてあるモーラ音高をいい感じにAIで推定してくれる」になります。

コアはエンジン内部詳細であるため、推定や推論がエンジンの責務なのかコアの責務なのかも内部詳細です。
なので replace_mora_pitch() が caller に負う責務である「モーラ音高の推定（とアクセント句系列属性の更新）」が関数docstringには正しいと感じます。
どうでしょうか？

[Hiroshiba]

どの記法ルールがどの実装に対応しているか初見では理解できなかったため、ルールとの対応づけをコメントに示す意図でした。
同時に、「規則がまずありそれを実装する」タイプのコード（例: parser）だと規則-実装対応づけに関するコメントは可読性に資すると感じます。
なんらかの方法でこの対応付けコメントは必要だと思うのですが、コメント形式のいいアイデアが何かあるでしょうか？

なるほどです。ルールから入った人には読みやすいけど、コードから入った人には読みづらくなるので、そこの塩梅かなと。

ファイル冒頭にルールを短文にして転写してRuleを振り、# Rule1: hogehogeと書くとかでしょうか。長いのが短くなるだけでだいぶ見やすくなるかなと！

• 全てのカナはカタカナで記述される
  • 読みはカタカナのみ（これがコメントに出ることはなさそう）
• アクセント句は/または、で区切る。、で区切った場合に限り無音区間が挿入される。
  • /で区切り、、で無音付き区切り
• カナの手前に_を入れるとそのカナは無声化される
  • _で無声化
• アクセント位置を'で指定する。全てのアクセント句にはアクセント位置を 1 つ指定する必要がある。
  • 'でアクセント位置
  • アクセント位置は１つだけ（別ルールのがわかりやすそう）
• アクセント句末に？(全角)を入れることにより疑問文の発音ができる
  • ？で疑問文

[Hiroshiba]

コアはエンジン内部詳細であるため、推定や推論がエンジンの責務なのかコアの責務なのかも内部詳細です。
なので replace_mora_pitch() が caller に負う責務である「モーラ音高の推定（とアクセント句系列属性の更新）」が関数docstringには正しいと感じます。
どうでしょうか？

なるほどです！
少なくともここはbaseクラスなので、実装を想定しない「更新」のみにしておくのが合ってる気がします。
ここがbaseクラスじゃなかったとした場合は･･･内部情報が重要だったら書きますが、まあ推論が裏で走ってることはエンジン開発者にとって重要な情報ではないので、書かなくても良いかなと思いました。
大事なのはパラメータが何らかの値で破壊的に変更される点かなと！

追記：あ、機械学習のアプリ応用のOSSコードを書いていて思ったのですが、裏で何が行われてるのかとかは全く気にされないです。値を渡したら値が返ってくる普通の関数と変わらない感じかなと。大事なのは重いか重くないか、並列処理は可能なのか、くらい！

[tarepan]

ルールから入った人には読みやすいけど、コードから入った人には読みづらくなる

👍
まさにこれですね。ずばりな言語化だと感じました。

ファイル冒頭にルールを短文にして転写してRuleを振り、# Rule1: hogehogeと書く

👍
この方式を採用して新たにPRを立てます。

[tarepan]

ここはbaseクラスなので、実装を想定しない「更新」のみ ...
推論が裏で走ってることはエンジン開発者にとって重要な情報ではないので、書かなくても良い ...
大事なのはパラメータが何らかの値で破壊的に変更される点

👍
確かに「推論してる」ということでさえ内部実装ですね。納得です。

機械学習 ... 値を渡したら値が返ってくる普通の関数と変わらない感じ ...
大事なのは重いか重くないか、並列処理は可能なのか、くらい！

本質情報ですね…😂