ユーザー辞書のインポート、エクスポートをUIに追加 #676

My-MC · 2023-04-24T14:01:58Z

内容

ユーザー辞書のインポート、エクスポートをUIに追加します。

~~UIはSettingのUIに追加する形にして、JavaScriptはかかないようにしました。~~

~~またread_dict関数のパーズ処理を違う関数に切り出し、利用するようにしました。~~
~~理由といたしましては、物理ファイルのようにpathlibが使えないためデータを渡してあげる感じになりました。~~

実際にファイルのエクスポート、インポート、overrideも行えることを確認しました。

スクリーンショット・動画など

その他

遅くなってしまい申し訳ございませんでした。

github-actions · 2023-04-24T14:13:06Z

Coverage Result

Resultを開く

Name	Stmts	Miss
voicevox_engine/init.py	2	1
voicevox_engine/acoustic_feature_extractor.py	75	0
voicevox_engine/dev/synthesis_engine/init.py	3	1
voicevox_engine/dev/synthesis_engine/mock.py	36	2
voicevox_engine/downloadable_library.py	93	5
voicevox_engine/full_context_label.py	162	3
voicevox_engine/kana_parser.py	86	1
voicevox_engine/metas/Metas.py	33	0
voicevox_engine/metas/MetasStore.py	29	14
voicevox_engine/metas/init.py	2	0
voicevox_engine/model.py	160	9
voicevox_engine/mora_list.py	5	0
voicevox_engine/part_of_speech_data.py	5	0
voicevox_engine/preset/Preset.py	12	0
voicevox_engine/preset/PresetError.py	3	1
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	201	159
voicevox_engine/synthesis_engine/make_synthesis_engines.py	59	51
voicevox_engine/synthesis_engine/synthesis_engine.py	130	11
voicevox_engine/synthesis_engine/synthesis_engine_base.py	67	9
voicevox_engine/user_dict.py	144	11
voicevox_engine/utility/init.py	5	0
voicevox_engine/utility/connect_base64_waves.py	37	0
voicevox_engine/utility/core_version_utility.py	9	2
voicevox_engine/utility/mutex_utility.py	11	1
voicevox_engine/utility/path_utility.py	26	8
TOTAL	1518	291

My-MC · 2023-04-30T05:55:51Z

テストの追加の必要がないことと、そのほかの不具合がないことが自分が見た中ではないことを確認しました。

takana-v

遅くなって申し訳ありません。
コメントを追加しておきました。

voicevox_engine/user_dict.py

ui_template/ui.html

run.py

takana-v · 2023-04-30T08:22:31Z

少し考えてみたのですが、辞書のコストが/user_dictではpriority表記になっており、cost表記になっている実ファイルを使った方が良い可能性があるので一旦レビューを取り下げました。

/user_dictを使った方がコード的にはかなりスッキリするのですが、
（user_dict.pyの変更と/download_dictの追加が不要になる）
もし品詞ごとに設定しているpriorityに紐づくcostが大きく変更された場合、大きな影響を受けることになります。
/download_dictを使う場合は、costが近いpriorityに補正されるので、そこまで大きな影響は受けません。

個人的にはpriorityに紐づくcostの変更を行う可能性はほぼ無いと思うため、/user_dictを使う形式でも問題ないかなと思いましたがちょっと他の方の意見も伺いたいです。 @Hiroshiba

My-MC · 2023-04-30T08:47:07Z

@takana-v 様自分の方で検証を行って確かめました。まず/user_dictの返すjsonをそのままdownloadして、それを読み込ませたところ、おっしゃるとおりkeyエラーが発生し、そもそもの関数の書き換えが必要になるような形になる感じでした。
以下ログです。

Traceback (most recent call last):
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\uvicorn\protocols\http\h11_impl.py", line 373, in run_asgi
    result = await app(self.scope, self.receive, self.send)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\uvicorn\middleware\proxy_headers.py", line 75, in __call__
    return await self.app(scope, receive, send)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\fastapi\applications.py", line 208, in __call__
    await super().__call__(scope, receive, send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\applications.py", line 112, in 
__call__
    await self.middleware_stack(scope, receive, send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\errors.py", line 181, in __call__
    raise exc
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\base.py", line 53, in __call__
    async with anyio.create_task_group() as task_group:
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\anyio\_backends\_asyncio.py", line 662, in __aexit__
    raise exceptions[0]
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\base.py", line 30, in coro
    await self.app(scope, request.receive, send_stream.send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\cors.py", line 92, in __call__
    await self.simple_response(scope, receive, send, request_headers=headers)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\middleware\cors.py", line 147, 
in simple_response
    await self.app(scope, receive, send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\exceptions.py", line 82, in __call__
    raise exc
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\routing.py", line 656, in __call__
    await route.handle(scope, receive, send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\routing.py", line 259, in handle
    await self.app(scope, receive, send)
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\routing.py", line 61, in app   
    response = await func(request)
               ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\fastapi\routing.py", line 226, in app    
    raw_response = await run_endpoint_function(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\fastapi\routing.py", line 161, in run_endpoint_function
    return await run_in_threadpool(dependant.call, **values)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\starlette\concurrency.py", line 39, in run_in_threadpool
    return await anyio.to_thread.run_sync(func, *args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\anyio\to_thread.py", line 31, in run_sync    return await get_asynclib().run_sync_in_worker_thread(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\anyio\_backends\_asyncio.py", line 937, in run_sync_in_worker_thread
    return await future
           ^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\.venv\Lib\site-packages\anyio\_backends\_asyncio.py", line 867, in run
    result = context.run(func, *args)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\run.py", line 1105, in setting_post
    parse_dict(user_dictionary_file.file), override=allow_override
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user_name\Desktop\python\voicevox_engine\voicevox_engine\user_dict.py", line 129, in parse_dict
    word["priority"] = cost2priority(word["context_id"], word["cost"])
                                                         ~~~~^^^^^^^^
KeyError: 'cost'

Co-authored-by: takana-v <44311840+takana-v@users.noreply.github.com>

takana-v · 2023-04-30T09:46:44Z

テストが落ちているみたいですね。
coverallsの方の問題かもしれません。
lemurheavy/coveralls-public#1710

Hiroshiba · 2023-04-30T15:18:00Z

僕も @takana-v さんと同じ意見で、既存の/user_dictと/import_user_dictを使い回す形にすると良いかなと思いました！！

理由は経路の統合で、辞書のimport/exportはこの経路だけにしておくとメンテナンス性が上がりそうだなと思ったためです。
（今のところはpriorityとcostの変換だけですが）

ちなみに/user_dictはこれ、/import_user_dictはこれです！
どう利用すればよいかわからない等あればなんでも聞いていただければ！！ @My-MC

My-MC · 2023-05-03T00:18:46Z

@Hiroshiba 様 /import_user_dictを使うとなると、HTML内にJavaScriptを記述する必要ができて、メインのPython以外のスクリプトが増えることになり、初学者の方などの参入が難しくなります。僕としてはPython以外の言語をなるべく書かない方針のほうが良いと思うのですがどうでしょうか。

takana-v · 2023-05-03T00:26:15Z

@My-MC
これら2つのsuggestionのように記述することで、Javascriptの記述無しで/user_dictと/import_user_dictを使いまわす形にできると思います。
（もしこの2つのsuggestionを適用してもエラーが出る場合は教えてください！）
#676 (comment)
#676 (comment)

My-MC · 2023-05-03T00:41:33Z

あ、/import_user_dictを使うということではなくDict[str, UserDictWord]の型を使うということでよろしいでしょうか。

takana-v · 2023-05-03T00:47:47Z

あ、ちょっと記述ミスをしていました。
使うのは/import_user_dictではなくimport_user_dict()ですね。（直接関数を実行するので）
/download_dictではなく/user_dictを用いてDict[str, UserDictWord]のデータを取得し、import_user_dict()関数を用いてインポートする、という認識です。

My-MC · 2023-05-03T00:49:56Z

なるほどわかりました。もしかしたら認識の行き違いが発生してるかもしれないのですが、 @Hiroshiba 様はどうですか。

takana-v · 2023-05-03T01:15:13Z

先ほどの@My-MC さんのコメントで気付きましたが、私の方に認識ミスが発生してましたね...申し訳ありません。
私の意見としては
・/download_dictではなく/user_dictを使いたい
・/import_user_dictに関しては、Javascriptを書く必要がありコードが複雑になりそうなら、内部の関数であるimport_user_dict()を実行する形式でも問題ない
（import_user_dict()を使うのを一カ所だけにすることによるメンテナンスコストの削減幅よりもJavascriptのコードのメンテナンスコストの方が大きいと感じます）
といった感じです。
ちょっとお手数おかけしますが、メンテナ判断をお願いしたいです。 @Hiroshiba

Hiroshiba · 2023-05-03T04:24:37Z

いくつか観点がありそうなのでまとめてみます。

・user_dictとdownload_dictで辞書のimport/export経路が違う
→少なくともuser_dictとdownload_dictは降ってくる辞書の形式が違うので統一すべきだと思います

・JavaScriptを書かないといけなくなる
→　user_dictとimport_user_dictを使う場合も、JS書かなくて良いはず･･･？（jsonをファイルとしてformで投げれる気がします）
まあそもそも、辞書がimportできたのか成否をユーザーに伝えてあげるコードを足したほうが良さそうだな～とちょっと思いました。

・user_dictとimport_user_dictを使い回すか、download_dictを足してsetting_postに機能を増やすか
→一般的に、同じ機能を持つAPIを増やすことはメンテナンスが大変になるので避けれるととても嬉しいです。
片方に機能実装するときもう片方にも実装が必要、それらが同じ挙動をすることの保証が必要、ドキュメントも増える、なぜ２つあるのか開発者が困惑する、仮に不要になっても消せない、などなど。
一方でJSコードを増やすのは、そのコードをメンテナンスすればOKなので、実はこちらのがだいぶ安い感じです･･･！

My-MC · 2023-05-03T06:23:37Z

・JavaScriptを書かないといけなくなる
→　user_dictとimport_user_dictを使う場合も、JS書かなくて良いはず･･･？（jsonをファイルとしてformで投げれる気がします）
まあそもそも、辞書がimportできたのか成否をユーザーに伝えてあげるコードを足したほうが良さそうだな～とちょっと思いました。

これなんですが使用上formで違うURL(エンドポイント)を使う場合、そのページにリダイレクトするのと、HTMLのコードが繰り返しの多い見ずらいコードになってしまいます。またそもそもbodyにそもままファイルを送っても処理ができません。実際にどちらも検証をし、確認しました。

ですので個人的には/import_user_dictは使わないほうがいいのかなと思いました。しかし、 @takana-v さんと @Hiroshiba でJavaScriptのメンテナンスコストの見積もりの違いがあるのも含めてissueを一回立ててみてもいいのかなと思いました。

Hiroshiba · 2023-05-04T07:03:06Z

検証してみました。たしかにHTMLのformだけで/import_user_dictを使うことはできなさそうでした。（POSTリクエスト、難しいですね･･･。）

良い方法があれば無理に/import_user_dictAPIを使わなくてもいいかなと思います。
ただ、もしsetting_postでimportする場合、辞書importがどうしても設定保存と同タイミングになるので、「インポートする辞書を保存する」のかと勘違いされるUIになりがちだろうなと思いました。

実際のUI

内部的には保存ボタンを押した時点で辞書を更新しているのですが、ユーザーの方が「エンジン起動時に一緒にインポートする辞書を保存した」のだと勘違いしちゃいそうだなと。
「インポートする」というボタンがあって「辞書を正常にインポートしました」と結果が表示されれば、勘違いしちゃうユーザーを減らせそうかもです。

`parse_dict`関数を削除

My-MC · 2023-05-14T10:36:58Z

ユーザ辞書の保存時にメッセージのトーストがでるようにしました。

画像のキャプチャー方法の都合上トーストが変なところに出てきてますが、通常は右下に表示されます。

Hiroshiba

トースト良いですね！！

まだちょっとユーザーに混乱されそうなUIになっちゃっていそうなのでコメントしてみました。

run.py

checkboxが正しく適用されるように

My-MC · 2023-08-13T10:24:53Z

今年に入り新しい環境でご助言をいただいてから時間が開いてしまい申し訳ありません。
辞書のimportをJavaScriptを使い/import_user_dictエンドポイントを使用するように変更を行いました。まだトーストの対応ができていませんがJavaScriptのコードの確認をお願いします。

Hiroshiba · 2023-08-14T19:24:49Z

@My-MC コード読ませていただきました！　すごくいい感じだと思います！！！
続きをぜひよろしくお願いします･･･！！

My-MC · 2023-08-19T10:35:04Z

辞書の保存のModalにも対応しました。機能の実装はこれですべてだと思います。レビューお願いします。

Hiroshiba

LGTM！！

ちょっとUIの細かいところだけ勝手に調整してコミットさせていただきました！！
ブートストラップのトーストの使い方の正しい方法は多分こっちだと思うのでそれも変えさせていただきました！

Hiroshiba · 2023-08-23T15:30:21Z

@takana-v さんか @y-chan さん、レビューお願いできると心強いです 🙇

Hiroshiba · 2023-08-23T15:34:30Z

設定UIが今生HTMLになってるので、Vuejsに変えられればと思ってissue立ててみました！

設定UIをVuejsにする #729

Hiroshiba · 2023-12-07T17:27:15Z

多分問題ないと思うのでマージさせていただきます！！
実装ありがとうございました！！

ユーザー辞書のインポート、エクスポートをUIに追加

2410392

My-MC requested a review from a team as a code owner April 24, 2023 14:01

My-MC requested review from y-chan and removed request for a team April 24, 2023 14:01

Merge branch 'VOICEVOX:master' into add_dictionary_ui

e201260

takana-v reviewed Apr 30, 2023

View reviewed changes

voicevox_engine/user_dict.py Outdated Show resolved Hide resolved

ui_template/ui.html Outdated Show resolved Hide resolved

takana-v reviewed Apr 30, 2023

View reviewed changes

run.py Outdated Show resolved Hide resolved

コメントの修正

2f8a17d

Co-authored-by: takana-v <44311840+takana-v@users.noreply.github.com>

エクスポートを/user_dictを使うように変更

2e4e627

My-MC added 2 commits May 6, 2023 09:02

JSONの読み込みができるように

086058f

`parse_dict`関数を削除

インポートボタンの追加と、インポート、保存メッセージを追加

ded87de

Hiroshiba reviewed May 14, 2023

View reviewed changes

run.py Outdated Show resolved Hide resolved

run.py Outdated Show resolved Hide resolved

My-MC added 2 commits August 12, 2023 11:52

user_dictionaryのprefixを追加しわかりやすくする

e97484f

辞書のimportをJavaScriptを使い/import_user_dictエンドポイントを使用するように

ef16fde

checkboxが正しく適用されるように

My-MC added 2 commits August 13, 2023 19:17

Remove unused function and class

1634b4e

重複したmessageを削除

9dafcc9

Merge branch 'master' into add_dictionary_ui

20fd572

辞書の保存のModalにも対応

7d7ee7f

いろいろjavascriptを調整

42163c3

Hiroshiba approved these changes Aug 23, 2023

View reviewed changes

Hiroshiba merged commit f8d6acf into VOICEVOX:master Dec 7, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ユーザー辞書のインポート、エクスポートをUIに追加 #676

ユーザー辞書のインポート、エクスポートをUIに追加 #676

My-MC commented Apr 24, 2023 •

edited

Loading

github-actions bot commented Apr 24, 2023 •

edited

Loading

My-MC commented Apr 30, 2023

takana-v left a comment

takana-v commented Apr 30, 2023 •

edited

Loading

My-MC commented Apr 30, 2023

takana-v commented Apr 30, 2023

Hiroshiba commented Apr 30, 2023

My-MC commented May 3, 2023 •

edited

Loading

takana-v commented May 3, 2023

My-MC commented May 3, 2023

takana-v commented May 3, 2023

My-MC commented May 3, 2023

takana-v commented May 3, 2023

Hiroshiba commented May 3, 2023 •

edited

Loading

My-MC commented May 3, 2023

Hiroshiba commented May 4, 2023 •

edited

Loading

My-MC commented May 14, 2023

Hiroshiba left a comment •

edited

Loading

My-MC commented Aug 13, 2023 •

edited

Loading

Hiroshiba commented Aug 14, 2023

My-MC commented Aug 19, 2023 •

edited

Loading

Hiroshiba left a comment

Hiroshiba commented Aug 23, 2023

Hiroshiba commented Aug 23, 2023

Hiroshiba commented Dec 7, 2023

ユーザー辞書のインポート、エクスポートをUIに追加 #676

ユーザー辞書のインポート、エクスポートをUIに追加 #676

Conversation

My-MC commented Apr 24, 2023 • edited Loading

内容

関連 Issue

スクリーンショット・動画など

その他

github-actions bot commented Apr 24, 2023 • edited Loading

Coverage Result

My-MC commented Apr 30, 2023

takana-v left a comment

Choose a reason for hiding this comment

takana-v commented Apr 30, 2023 • edited Loading

My-MC commented Apr 30, 2023

takana-v commented Apr 30, 2023

Hiroshiba commented Apr 30, 2023

My-MC commented May 3, 2023 • edited Loading

takana-v commented May 3, 2023

My-MC commented May 3, 2023

takana-v commented May 3, 2023

My-MC commented May 3, 2023

takana-v commented May 3, 2023

Hiroshiba commented May 3, 2023 • edited Loading

My-MC commented May 3, 2023

Hiroshiba commented May 4, 2023 • edited Loading

My-MC commented May 14, 2023

Hiroshiba left a comment • edited Loading

Choose a reason for hiding this comment

My-MC commented Aug 13, 2023 • edited Loading

Hiroshiba commented Aug 14, 2023

My-MC commented Aug 19, 2023 • edited Loading

Hiroshiba left a comment

Choose a reason for hiding this comment

Hiroshiba commented Aug 23, 2023

Hiroshiba commented Aug 23, 2023

Hiroshiba commented Dec 7, 2023

My-MC commented Apr 24, 2023 •

edited

Loading

github-actions bot commented Apr 24, 2023 •

edited

Loading

takana-v commented Apr 30, 2023 •

edited

Loading

My-MC commented May 3, 2023 •

edited

Loading

Hiroshiba commented May 3, 2023 •

edited

Loading

Hiroshiba commented May 4, 2023 •

edited

Loading

Hiroshiba left a comment •

edited

Loading

My-MC commented Aug 13, 2023 •

edited

Loading

My-MC commented Aug 19, 2023 •

edited

Loading