ENH: pydanticをv2へ #1184
ENH: pydanticをv2へ #1184
Conversation
Coverage ResultResultを開く
|
sabonerune
force-pushed
the
maint/pydantic-v2
branch
2 times, most recently
from
fbddb05 to
280e867
Compare
|
バグが直るの楽しみです…！！ |
sabonerune
force-pushed
the
maint/pydantic-v2
branch
from
280e867 to
0e9f059
Compare
sabonerune
force-pushed
the
maint/pydantic-v2
branch
2 times, most recently
from
85435d2 to
a111f9a
Compare
sabonerune
force-pushed
the
maint/pydantic-v2
branch
2 times, most recently
from
9c10495 to
c9aa572
Compare
sabonerune
force-pushed
the
maint/pydantic-v2
branch
from
c9aa572 to
66c6ff1
Compare
|
OpenAPIスキーマーの問題は恐らく仕様らしいのでドラフトを解除します。 |
|
Swagger UIが壊れる問題に対処するため |
There was a problem hiding this comment.
- 👍️ 移行ガイドに沿い、適切に関数・クラスを変更しています。good work!
- 👍️ 移行以外の変更を極力控え、レビューしやすい変更です。great!
- 変数名をより明確にし、更に可読性を向上できます
- 機能追加・変更か否か、確認が必要な箇所があります
レビュー用情報
- リネーム
.dict()→.model_dump()(migration docs).validate()→.model_validate()(docs).copy()→.model_copy()(migration docs)
- 置き換え
parse_obj_as()→TypeAdapter(migration docs)| None→| SkipJsonSchema[None]
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "null" |
There was a problem hiding this comment.
"required": false でなく "anyOf null" になっているように見えます。
意図的な変更でしょうか？
There was a problem hiding this comment.
Pydantic V2の仕様変更で出力されるスキーマーが正確になったという感じです。
There was a problem hiding this comment.
スキーマが正確になったのではなく、マイグレーション不足で意図しないスキーマ変更が起きていると考えます（以下、検証と考察）。
スキーマに diff が入っている UpdateInfo.contributors 属性について、list[str] | None を list[str] | SkipJsonSchema[None] とすると diff が無くなりました。
これは PR 冒頭にあるとおり、: X | None を Nullable と解釈する仕様変更に伴うものです。
このマイグレーションをしない場合、新スキーマは required: false かつ Nullable になり、今までの「field に値を入れる or field ごと無い」から「field に値を入れる or field に Null を入れる or field ごと無い」へ VOICEVOX API の仕様変更が起きます。
ゆえに SkipJsonSchema を用いてマイグレーションすべき箇所と考えます。
There was a problem hiding this comment.
何を正しいとするかだと思います。
V1→V2の変更で、今のPython実装に合うようになります。
例えばpause_moraはrequired=falseですが、
実態としてnoneが返ってきてました。。
There was a problem hiding this comment.
今のPython実装に合うようになります
👍️
スキーマと実装に乖離があるのですね、現状を把握しました。
@Hiroshiba
スキーマを実態に合わせて変更することは仕様変更（update or fix）です。
リファクタリングの一種である bump PR で仕様変更をするのはバットプラクティスです（悪影響例: update_infos.json 作るときに 開発環境の向上 と誤認して記載を忘れる）。
仕様変更は後続 PR でおこなうのが適切と考えます。
There was a problem hiding this comment.
ふと思い出したのですがFastAPIがpydantic V2のModelと同じ挙動になるのはないとおもいます。
というのもseparate_input_output_schemasという形でPydanticの入力と出力の対応が異なるという問題に対処しているからです。
今回のPRではこれをTrueにしてしまうとスキーマーが大きく変わってしまうためFalseにしてあります。
There was a problem hiding this comment.
ちゃんとわかってないかもなのですが、separate_input_output_schemas=trueなら同じ挙動になる気がします。
PydanticはV2であれV1であれ「キーがない」という型を表現できないんですよね。
APIの入力側はこの仕様でも問題ないはず。
pydanticに合わせてvalue=nullがAPIに投げられてもOK、投げなくてもOKにすれば良いだけなので。
問題は出力側で、pydanticは必ず全キーがあるモデルしか定義できないから、出力は必ずvalue=nullになる。
この入出力のずれの折衷案がFastAPIのseparate_input_output_schemas=trueで、実装をpydanticに合わせた形だと思います。
逆にV2&separate_input_output_schemas=falseやV1のときはスキーマと実装がずれちゃってます。
よくよく考えると、別にこの辺りでpydanticがV1からV2になって変わったことはないはずで、V2に対応するときにFastAPI側がバグっぽい挙動を直したって感じな気がしますねぇ
There was a problem hiding this comment.
pydanticは必ず全キーがあるモデルしか定義できないから、出力は必ず
value=nullになる。
pydantic にはこれに対応するための仕組みがあります。
from pydantic import BaseModel, Field
from pydantic.json_schema import SkipJsonSchema
class APIModel(BaseModel):
required: int
not_required: int | SkipJsonSchema[None] = None
api_model = APIModel(required=10)
print(api_model)
# required=10 not_required=None
api_model_dict = api_model.model_dump()
print(api_model_dict)
# {'required': 10, 'not_required': None}
api_model_dict = api_model.model_dump(exclude_none=True)
print(api_model_dict)
# {'required': 10}There was a problem hiding this comment.
それは･･･noneをexcludeしているのであって、キーがないのを表現できてるわけではない気がします！
実際
- RequiredでNoneが入りうる型
- NonRequiredでNoneが入らない型
の２つが同時に満たせないかなと
There was a problem hiding this comment.
- RequiredでNoneが入りうる型
- NonRequiredでNoneが入らない型
の２つが同時に満たせないかなと
一応できた？
from typing import Annotated, Any
from pydantic import (
BaseModel,
ConfigDict,
Field,
ValidationInfo,
ValidatorFunctionWrapHandler,
WrapValidator,
)
from pydantic.json_schema import SkipJsonSchema
from pydantic_core import PydanticCustomError
def non_null_validator(
v: Any, handler: ValidatorFunctionWrapHandler, info: ValidationInfo
) -> Any:
result = handler(v)
if result is None:
raise PydanticCustomError(
"null_error",
'"{field_name}" is optional but cannot be null.',
{"field_name": info.field_name},
)
return result
class Model(BaseModel):
model_config = ConfigDict(validate_assignment=True)
required: int
optional: Annotated[
int | SkipJsonSchema[None],
WrapValidator(non_null_validator),
] = Field(default=None)
required_nullable: int | None
optional_nullable: int | None = Field(default=None)
try:
Model(required=10, optional=None)
except ValueError as e:
print(e)
# 2 validation errors for Model
# optional
# "optional" is optional but cannot be null. [type=null_error, input_value=None, input_type=NoneType]
# required_nullable
# Field required [type=missing, input_value={'required': 10, 'optional': None}, input_type=dict]
# For further information visit https://errors.pydantic.dev/2.7/v/missing
m = Model(required=10, required_nullable=None, optional_nullable=None)
print(m)
# required=10 optional=None required_nullable=None optional_nullable=None
m.optional = 5
try:
m.optional = None
except ValueError as e:
print(e)
# optional
# "optional" is optional but cannot be null. [type=null_error, input_value=None, input_type=NoneType]
del m.optional
print(m)
# required=10 required_nullable=None optional_nullable=None
print(m.model_dump_json(exclude_unset=True))
# {"required":10,"required_nullable":null,"optional_nullable":null}del m.optionalが怪しいですが…
あとexclude_none=Trueやexclude_unset=True両方に言えるのですがこの設定はFastAPIのエンドポイントやpydanticのdump時に指定するものでモデルそのものに設定できないといのも問題だと思います。
(カスタムシリアライザを使えば表現可能な気がしますが管理が面倒だと思います。)
また、既存のモデルにこのnon_null_validatorを入れると保存しておいた以前のエンジンで生成したクエリをそのまま新しいエンジンに投げるとエラーになるという実質的な破壊的変更が起こります。
| def pydantic_to_native_type(value: Any) -> Any: | ||
| """pydanticの型をnativeな型に変換する""" | ||
| return json.loads(json.dumps(value, default=pydantic_encoder)) | ||
| return jsonable_encoder(value) |
| priority: Annotated[ | ||
| int | None, | ||
| int | SkipJsonSchema[None], | ||
| Query( | ||
| ge=MIN_PRIORITY, | ||
| le=MAX_PRIORITY, | ||
| description="単語の優先度（0から10までの整数）。数字が大きいほど優先度が高くなる。1から9までの値を指定することを推奨", | ||
| # "SkipJsonSchema[None]"の副作用でスキーマーが欠落する問題に対するワークアラウンド | ||
| json_schema_extra={"maximum": MAX_PRIORITY, "minimum": MIN_PRIORITY}, |
There was a problem hiding this comment.
入力側はSkipJsonSchemaを外しても破壊的変更になってないので外して良さそう
| type: StyleType | None = Field( | ||
| type: StyleType = Field( |
There was a problem hiding this comment.
あっ ここが| Noneなのは互換性のためで、昔はこのキーなかったんですよね･･･。
ということで必要かもです。。。
･････････････あれ、いやこれはこの形が以前から正しいのか･･･････。
ん？？？
あれじゃあpydanticを使っている以上、互換性維持したままキーの追加が本質的にできない･･･････？
だいぶ混乱してきた、よくわかんないですね！！！ 😇
内容
Pydanticを2.7へ更新します。ref: Migration Guide - Pydantic
現状
FastAPIのバグが原因と思われる問題があるためドラフトにしています。関連 Issue
その他
現状の問題
特に問題になりそうなのは以下の二つです。
OpenAPIスキーマーがおかしい
オプションのクエリやフォームで
nullが使用可能と記述されています。これは多分誤りです。
->
pydantic.json_schema.SkipJsonSchemaで一応回避が可能。Swagger UIの表示が壊れている
上記が原因でドキュメントのUIが壊れています。
例
その他気になる挙動
test_post_user_dict_word_422.jsonでPydanticの情報まで出てきている?これは意図的?FastAPI 0.110.2で解消
separate_input_output_schemas=Trueにしたときドキュメントの記載と動作が異なる?Falseに指定したため影響はないref: Separate OpenAPI Schemas for Input and Output or Not - FastAPI