wtforms also allows a str as a valid choice of a SelectField

[jkittner]

As far as I can see, choices of a SelectField also support an Iterable of just a strs, not only a tuple[Any, str].

SelectField(choices=["foo", "bar"])

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

[github-actions[bot]]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[jkittner]

Mhm, not sure why it does not like Iterable[_Choice | str]

This seems to work (explicitly defining unions of tuple[str, ...] | list[str]), but it is quite ugly...

(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] = (),

[srittau]

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.

[Daverball]

pyright's behavior seems very broken to me. Why is it reporting a dict instead of a Callable as the outermost type? Why is the key type _Choice? That's not one of the possible options. @erictraut Can you figure out what's happening here?

[erictraut]

Pyright's behavior looks correct here to me. The parameter type is Iterable[_Choice]. You appear to have strict-mode type checking enabled, so pyright's reportUnknownArgumentType check is enabled. Let's take a look at a specific case where this error is triggered:

SelectField(choices={"a": (("", "", {}),)})

Here the specified value is an Iterable[str], which is assignable to Iterable[_Choice], but the value type argument for the dict cannot be fully inferred from either the bidirectional inference context (Iterable[str]) or from the value expression ((("", "", {}),)). The {} subexpression evaluates to a type of dict[Unknown, Unknown], which makes the resulting argument type "partially unknown".

[Daverball]

That makes sense, I hadn't considered that through the addition of str, dict[str, Any] would pass as Iterable[str] and in turn Iterable[_Choice], so type checkers would be able to infer the type of the lambda based on that, rather than _GroupedChoices/dict[str, Any]. One could make the argument that a type checker should still infer based on dict[str, Any] since it is closer to dict than Iterable[str], but that of course quickly gets very complex.

I knew about the str shortcut for WTForms and I decided against including it when first writing these stubs because it seemed like a recipe for disaster and it appears my intuition was at least partially correct. We could potentially make this work with a separate overload for the str case, but that makes subclassing these fields far more annoying and it also makes the type alias less useful, so I'm not fully convinced it is worth it, unless all type checkers can pass the regression tests with the current, more simple change.

[srittau]

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 Any), as string shortcuts are seemingly a desired feature.

[Daverball]

Is passing an empty dict something that happens in real code? If it isn't, we could just adapt our tests.

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. Iterable[str] does worry me a little for other reasons, like accepting str or preventing flagging things like (('', '')) as an error, since it was meant to be a single blank choice rather than two blank choices.

EDIT: It would also start accepting arbitrary Mapping[str, ...], which may lead to confusion, since you will get the individual group names as choices rather than grouped choices.

[JelleZijlstra]

Any updates here? CI is still failing, and it seems like there are some suggestions above that should make things work.

[JelleZijlstra]

Thanks for contributing! I'm closing this PR for now, because it still fails some tests after several months of inactivity. If you are still interested, please feel free to open a new PR (or ping us to reopen this one).