-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
add default coerce to any Select2Field
#2784
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
base: master
Are you sure you want to change the base?
Conversation
|
Why do you need to change typing? Do we need to change all other orm libraries? |
what do you mean? i'm not changing the typing. This PR just extract the right |
|
|
||
|
|
||
| class T_FIELD_ARGS_VALIDATORS_SELECTABLE(T_FIELD_ARGS_VALIDATORS_ALLOW_BLANK): | ||
| coerce: NotRequired[t.Callable[[t.Any], t.Any]] |
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.
I think we can simply add coerce to T_FIELD_ARGS_VALIDATORS and remove T_FIELD_ARGS_VALIDATORS_SELECTABLE
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.
I think we can simply add coerce to
T_FIELD_ARGS_VALIDATORSand removeT_FIELD_ARGS_VALIDATORS_SELECTABLE
I've tried your suggestion but unfortunately it all fields that don't include coerce attr. will be affected and raise typing issues. I believe T_FIELD_ARGS_VALIDATORS_SELECTABLE is more clear and dedicated to SELECT fields
|
Sorry, let me be more specific. Why not editing |
the |
| return value if value else None | ||
|
|
||
|
|
||
| def coerce_factory(type_: TypeEngine[t.Any]) -> t.Callable[[t.Any], t.Any]: |
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.
Does this PR only make it work for sqlalchemy? Or other libraries do not have this issue?
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.
it is tested only in Sqlalchemy, however, i've tried to add a Peewee test but it seems like there is no way to use coerce function in Peewee or Pymongo
run-time error is raised if Select2Field did'nt include
coerce=intin form_argsHere is how to replicate the error
This PR adds a default coerce function if it is not provided explicitly based on the Column's type
the added tests won't pass without changes in this PR