-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
wtforms also allows a str as a valid choice of a SelectField #12203
base: main
Are you sure you want to change the base?
Conversation
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Mhm, not sure why it does not like This seems to work (explicitly defining unions of (not formatted with black so the diff is easier to see) diff --git a/stubs/WTForms/wtforms/fields/choices.pyi b/stubs/WTForms/wtforms/fields/choices.pyi
index acbb01dc9..0c802cc50 100644
--- a/stubs/WTForms/wtforms/fields/choices.pyi
+++ b/stubs/WTForms/wtforms/fields/choices.pyi
@@ -7,7 +7,7 @@ from wtforms.form import BaseForm
from wtforms.meta import DefaultMeta, _SupportsGettextAndNgettext
# technically this allows a list, but we're more strict for type safety
-_Choice: TypeAlias = tuple[Any, str] | tuple[Any, str, dict[str, Any]] | str
+_Choice: TypeAlias = tuple[Any, str] | tuple[Any, str, dict[str, Any]]
# it's too difficult to get type safety here due to to nested partially invariant collections
_GroupedChoices: TypeAlias = dict[str, Any] # Any should be Collection[_Choice]
_FullChoice: TypeAlias = tuple[Any, str, bool, dict[str, Any]] # value, label, selected, render_kw
@@ -51,7 +51,7 @@ class SelectField(SelectFieldBase):
label: str | None = None,
validators: tuple[_Validator[_FormT, Self], ...] | list[Any] | None = None,
coerce: Callable[[Any], Any] = ...,
- choices: Iterable[_Choice] | _GroupedChoices | Callable[[], Iterable[_Choice] | _GroupedChoices] | None = None,
+ choices: Iterable[_Choice] | list[str] | tuple[str, ...] | _GroupedChoices | Callable[[], Iterable[_Choice] | tuple[str, ...] | list[str] | _GroupedChoices] | None = None,
validate_choice: bool = True,
*,
filters: Sequence[_Filter] = (), |
It seems that due to the new union element, pyright is not able to statically determine the type of the empty dict in the tests anymore. I don't know know why that is. |
pyright's behavior seems very broken to me. Why is it reporting a |
Pyright's behavior looks correct here to me. The parameter type is SelectField(choices={"a": (("", "", {}),)}) Here the specified value is an |
That makes sense, I hadn't considered that through the addition of I knew about the |
Is passing an empty dict something that happens in real code? If it isn't, we could just adapt our tests. Otherwise I think using overloads is the only solution I see (other than using |
That's a fair point, it's not really useful to pass an empty dict, since it's the same as omitting it in the first place. I'd be fine with adapting the tests. EDIT: It would also start accepting arbitrary |
As far as I can see,
choices
of aSelectField
also support an Iterable of just astr
s, not only atuple[Any, str]
.This is apparently called "shortcut" in the tests
This is also tested:
https://github.com/wtforms/wtforms/blob/8211fc854f91aef184b6fc8766d7dad14b198387/tests/fields/test_select.py#L141-L152
...and as a return value of a callable
https://github.com/wtforms/wtforms/blob/8211fc854f91aef184b6fc8766d7dad14b198387/tests/fields/test_selectmultiple.py#L61
I am not 100% sure that's the correct fix? Happy to hear your feedback - I'm a newbie at typeshed