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

非推奨化されたdistutils.versionへの依存をなくし、python-semverに移行する #609

Merged
merged 19 commits into from Apr 15, 2023

Conversation

aoirint
Copy link
Member

@aoirint aoirint commented Feb 2, 2023

内容

PEP 632で非推奨化された、distutilsへの依存をなくします。

コアのmetasからバージョンをパースし、最新のコアを選択するためにdistutils.version.LooseVersionが使われていました。

代替として、packaging.version.parseに切り替えました。 (切り替え先をpython-semverに変更)
代替として、python-semverに切り替えました。

また、最新コアの選択ロジックをutility.core_versionに切り出し、テストを追加しました(他に追加した方がよさそうなテストケース/あり得るコアのバージョンがあれば教えてください)。

packaging.version.parseの戻り値の型がpackaging.version.Version | packaging.version.LegacyVersionになっていて、packaging.version.LegacyVersionが含まれるsetuptools/packagingのバージョンがあるかもしれませんが、
削除されていると判断してpackaging.version.Versionのみとみなしています(このあたりのパッケージのバージョン管理がどうなっているかよくわからないです...)。
(切り替え先をpython-semverに変更)

関連 Issue

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

その他

@aoirint aoirint requested a review from a team as a code owner February 2, 2023 03:20
@aoirint aoirint requested review from y-chan and removed request for a team February 2, 2023 03:20
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Coverage Result

Resultを開く
Name Stmts Miss Cover
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 75 0 coverage-100%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 36 2 coverage-94%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/metas/Metas.py 33 0 coverage-100%
voicevox_engine/metas/MetasStore.py 29 14 coverage-52%
voicevox_engine/metas/init.py 2 0 coverage-100%
voicevox_engine/model.py 150 9 coverage-94%
voicevox_engine/mora_list.py 4 0 coverage-100%
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 201 159 coverage-21%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 57 49 coverage-14%
voicevox_engine/synthesis_engine/synthesis_engine.py 130 11 coverage-92%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 67 9 coverage-87%
voicevox_engine/user_dict.py 144 11 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 1407 279 coverage-80%

@aoirint aoirint mentioned this pull request Feb 2, 2023
3 tasks
test/test_core_version.py Outdated Show resolved Hide resolved
test/test_core_version.py Outdated Show resolved Hide resolved
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.

PRありがとうございます!!
いくつかコメントしてみましたが、方針は賛成です!

@Hiroshiba
Copy link
Member

@aoirint こちらどうでしょう 👀

@aoirint
Copy link
Member Author

aoirint commented Apr 8, 2023

遅くなりましたが返答しました!

@aoirint aoirint changed the title 非推奨化されたdistutils.versionへの依存をなくし、packaging.versionに移行する 非推奨化されたdistutils.versionへの依存をなくし、python-semverに移行する Apr 9, 2023
@aoirint aoirint requested a review from Hiroshiba April 9, 2023 15:01
@aoirint
Copy link
Member Author

aoirint commented Apr 9, 2023

移行先をpackagingからpython-semverに変更しました。再レビューおねがいします!

  • core_version_utilityのバージョン文字列処理をpackagingからpython-semverに変更
    • poetry add semver
    • requirements*.txtの更新
  • テストに0.14.0 < 0.15.0-preview.XXXを確認するコードを追加

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!!

PRありがとうございます!
こうやってちょっとずつ綺麗にしていくの、大事なのですがなかなか手が回っていないのでとても嬉しいです・・・!

voicevox_engine/utility/__init__.py Outdated Show resolved Hide resolved
Copy link
Member

@takana-v takana-v left a comment

Choose a reason for hiding this comment

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

1点コメントしましたが、LGTMです。
Poetryのlockファイルがコンフリクトしているみたいなので、解消の方をお願いします:bow:

test/test_core_version_utility.py Show resolved Hide resolved
@aoirint
Copy link
Member Author

aoirint commented Apr 14, 2023

コンフリクト

解消しました。

poetry.lockcontent-hashがコンフリクトしていたので、masterブランチとマージし、差分が最小になる形(poetry lock --no-update)でcontent-hashを更新しました。

@Hiroshiba
Copy link
Member

ありがとうございます! たぶん大丈夫だと思うのでマージします!!

@Hiroshiba Hiroshiba merged commit 69c77cc into VOICEVOX:master Apr 15, 2023
3 checks passed
@aoirint aoirint deleted the patch-remove-distutils branch April 15, 2023 00:20
@aoirint aoirint restored the patch-remove-distutils branch April 15, 2023 00:27
@aoirint aoirint deleted the patch-remove-distutils branch October 9, 2023 02:58
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.

None yet

3 participants