-
Notifications
You must be signed in to change notification settings - Fork 121
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
feat!: [Python] Enum
をLiteral
に
#950
Conversation
61507ca
to
5a70552
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typescriptのenumはともかく、PythonのEnumは実は問題ないかも?とか少し思いました!
Literal stringは型の意味しかないですが、Enumクラス自体は値なのでどっちかというとこっちのが便利ではありそう。
例えばexample/python/run.py
はたぶんchoices
を使ってこうも書けます:
argparser.add_argument(
"--mode",
default="AUTO",
type=StyleType,
choices=StyleType.__members__.values()
help='モード',
)
あとバリデーションもこれで可能です:
style: str = "hoge"
style_ = StyleType(style)
# ValueError: 'hoge' is not a valid StyleType
内部で雑に使うliteralだったらliteral stringで良さそうだけど、公開API用だとEnumのが良いかも・・・?
まあでもそこまで強い意見じゃないです!
前方互換性のためみたいな感じだったような? from enum import Enum
class Color(Enum):
RED = 1
GREEN = 2
BLUE = 3 みたいだったときに if color == Color.RED:
...
elif color == Color.GREEN:
...
else: # color == Color.BLUE:
... と書く人が多く、それだと新しく何かを追加したときに既存のコードが壊れて互換性破壊になります。 そこで「今後増える可能性がある」というのを明示するために「一応パターンは決まってるけど必ずしも全部じゃ無いよ(だから必ずelse節を作ってね、など)」というのを表したく、そのためにTSでいう ...というのが #941 の意図という認識です。 |
なるほどです!! 一応Enumでも同じことはできる・・・けど、なんか微妙に複雑になりそうですねぇ・・・ class StyleType(str, Enum): # こっちがユーザー用
...
NonExhaustiveStyleType = StyleType | str # こっちはAPI用 あと StyleType: TypeAlias = Literal["talk", "sing"] | str
my_style: StyleType = "song" # singの間違い
直感、言語的なサポートなく 一旦 |
[追記] イメージはこんな感じ。 from enum import Enum
from typing import Any, Literal, NoReturn, TypeAlias
from pydantic import GetCoreSchemaHandler
from pydantic_core import CoreSchema, core_schema
class NonExhaustive(Enum):
pass
# 以下の二つは無くてもいいけど、少しでもましなエラーメッセージのため
@classmethod
def __get_pydantic_core_schema__(
cls, source_type: Any, handler: GetCoreSchemaHandler
) -> CoreSchema:
# TODO: pydantic/pydantic-core#1579 の機能がリリースされたらそれを使う
return core_schema.no_info_after_validator_function(
cls._no_input_allowed, core_schema.any_schema()
)
@classmethod
def _no_input_allowed(cls, _: object) -> NoReturn:
raise ValueError(f"No input is allowed for `{cls.__name__}`")
StyleType: TypeAlias = (
Literal["TALK", "SINGING_TEACHER", "FRAME_DECODE", "SING"] | NonExhaustive
) 例外メッセージはこう。 pydantic/pydantic-core#1579 を見るにこれが限度っぽい。 Traceback (most recent call last):
File "/home/ryo/src/github.com/VOICEVOX/voicevox_core/main/crates/voicevox_core_python_api/a.py", line 35, in <module>
x = StyleMeta(style_type="間違った文字列")
File "/home/ryo/.cache/pypoetry/virtualenvs/voicevox-core-hMLxFPJX-py3.10/lib/python3.10/site-packages/pydantic/_internal/_dataclasses.py", line 141, in __init__
s.__pydantic_validator__.validate_python(ArgsKwargs(args, kwargs), self_instance=s)
pydantic_core._pydantic_core.ValidationError: 2 validation errors for StyleMeta
style_type.literal['TALK','SINGING_TEACHER','FRAME_DECODE','SING']
Input should be 'TALK', 'SINGING_TEACHER', 'FRAME_DECODE' or 'SING' [type=literal_error, input_value='間違った文字列', input_type=str]
For further information visit https://errors.pydantic.dev/2.9/v/literal_error
style_type.function-after[_no_input_allowed(), any]
Value error, No input is allowed for `NonExhaustive` [type=value_error, input_value='間違った文字列', input_type=str]
For further information visit https://errors.pydantic.dev/2.9/v/value_error 一応Javaではこういう方法がいいと考えています。 public class StyleType {
public static final StyleType TALK = new StyleType();
public static final StyleType SINGING_TEACHER = new StyleType();
public static final StyleType FRAME_DECODE = new StyleType();
public static final StyleType SING = new StyleType();
private StyleType() {}
} ただPythonの方は、ここ↓まではいけてJSON → dataclass (via Pydantic)はできるけど、dataclass → JSONが詰んでしまうと思っています。何故なら純粋なdataclassにPydanticが介入できるポイントは無いので。 # Rust (PyO3)によるオブジェクト
class StyleType:
# 純Pythonだとここで詰むけど、PyO3ならいける…はず?
TALK: Final["StyleType"]
SINGING_TEACHER: Final["StyleType"]
FRAME_DECODE: Final["StyleType"]
SING: Final["StyleType"]
# Pydanticのため
def __new__(cls, s: str) -> "StyleType": ...
# 「この型のJSON表現はstringですよ」ということをPydanticに伝える
@classmethod
def __get_pydantic_core_schema__(cls, source_type: Any, handler: GetCoreSchemaHandler) -> CoreSchema: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちょっと事例がないか調べた感じ、typescriptライブラリで| string
が削除されてる例を見かけたのでメモ。削除する流れのが良さそう。
aws/aws-sdk-js-v3#4278
EnumからLiteralにする場合ですが、ドキュメントから削除されたりしませんか 👀
どういうtypeがあり得るのかドキュメント内でわかるようになってたのがわからなくなるなら、Enumに軍配が上がるかもです。
ドキュメントにはEnumであれLiteralであれ書かれない、もしくはどっちでも書かれるなら、まあLiteralで良さそう!
便利なのはクラスが実態として存在するEnumの方だけど(便利なメソッドがあるので)、定義の面倒くささとか考えるとまあLiteralでも。
NonExhaustiveの定義どれが良いかはちょっとわかんないですが、str
じゃなければあとはそこそこなんでも良さそう!
ちょっと話し反れるのですが、NonExhaustiveなものとそうじゃない型どっちも必要かも・・・?とか少し思いました。
特に手堅いコードを書きたいわけじゃく、サクッと動かしたい人向けに・・・。
まあでもメンテ大変だし、Rustと合わせる形で良いかも!
|
うーん。そもそも列挙型/直和型を何でもnon exhaustiveにすべきかというと、多分そうではない、というのはありますね。 状況によっては「どう取り繕おうがバリアントを増やした時点で破壊的変更」と言えてしまうこともあるとは思います。例えばあるAPIが以下のような #[non_exhaustive]
pub enum Outcome {
Completed,
Failed,
Canceled,
} これを使うユーザーは、こういうコードしか書けないはずです。こういうケースにおいてはしっかりとexhaustiveとし、バリアントを追加するときになったら潔く破壊的変更とする方が皆が幸福になると思います。 match library::api() {
Outcome::Completed => info!("OK!"),
Outcome::Faield => return Err(/* … */),
Outcome::Aborted => { /* retry */ }
outcome => {
panic!("unexpected result. what happened?????: {outcome:?}")
}
} で、このPRで対象にしているような列挙型は現在3つあります。
1.の 2.の 問題は3.の # 多分CとC++以外、列挙型の網羅性チェックはほぼあらゆる言語に備わっている
match style.type:
case "talk":
...
case "singing_teacher":
...
case "frame_decode":
...
case "sing":
...
すみません、こっちはちょっとあまりイメージできませんでした。 …関係あるかはわからないのですが、 [追記] あと今気付いたけど、 |
破壊的変更の考え方を調べてました。
あとGitHub Enterprise Cloud REST APIも参考になりそうでした。
これに従うとenumの値を追加するだけなら破壊的変更にならないかもです。 non_exhaustiveにすべきかと、破壊的変更として扱うべきかは別な気がします。 まとめると、既存の処理が変わらなければ破壊的変更として扱わない、enumの返り値は「それを無視している」コードが正しく動くなら破壊的変更ではない、
ちゃちゃっと試しに1回利用するだけのつもりなら「将来値が変わりうる」はどうでもよく、煩わしいこともあるだろうなというニュアンスでした! |
あ、逆で、
あ、まだちょっとわかっていなくて、返り値の型に |
ちょっとわかってないですけど、これら2つに あとはqryxipさんのコメントを見て考えが変わりました。
StyleTypeに値を追加することは破壊的変更には当たらない(API側の既存の挙動を変更しない)と思うので、StyleTypeの性質が
まあこれ結論として、 |
再度リテラル文字列と文字列Enum、どちらの方が良いか考えました。 まず今更気づいたのですが、初学者にとって型はぶっちゃけ理解できないと思います。 StrEnumはクラスなので「インポートしないといけない」という発想にたどり着かない。逆に言うと1回経験すればわかる。どこからインポートすればいいかはわかんないかもだけど。 あともう一度思い出すとawaitも分からないはず。全体的に、一番最初にQiita/Note記事を書こうと頑張る初学者がくじけるレベルの、かなり難しいコードになってそう。 まあでも全体的に優しくすべきというかなり抜本的な話になるので、全体の方針考えないといけず大変なっちゃう。 ということでまとめると、とりあえずLiteral文字列の方がよく、あとexampleをなるべく網羅的に用意してあげるのがいいのかなと思いました! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
crates/voicevox_core_python_api/python/voicevox_core/_models.py
Outdated
Show resolved
Hide resolved
Refs: VOICEVOX#950 (comment) Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
#941 の続き。また列挙型の網羅性についての議論は以下。 #950 (comment)
内容
目的としては二つ。一つは VOICEVOX/voicevox_engine#1413 と同じで、書く人、今回の場合つまりユーザーの利便性を高めるため。もう一つはこの後別PRで
| str
を付けてLiteral[…] | str
Literal[…] | _Reserved
とすることで、Rustで言うnon exhaustiveとし、後方互換性を確保できるようにするため。関連 Issue
See-also: #941 (comment)
その他