Add more selectable typings

[bryanforbes]

No description provided.

[ilevkivskyi]

Thanks for PR! I finally had time to test this against our codebase. This PR generates a bunch of new errors that may be false positives. Could you please double check? (And add tests, as usual.)

• Argument 1 to "limit" of "GenerativeSelect" has incompatible type "Optional[int]"; expected "int" and Argument 1 to "offset" of "GenerativeSelect" has incompatible type "Optional[int]"; expected "int". Could you please double-check that None doesn't work there?

• Argument 1 to "group_by" of "GenerativeSelect" has incompatible type "str"; expected "ClauseElement" and Argument 1 to "insert" of "TableClause" has incompatible type "Dict[Column[Any], int]"; expected "Optional[Mapping[str, Any]]". I noticed that there are some legacy APIs that allow str (but they generate warning that recommends wrapping it in text(...)) and bool where ClauseElement is expected. And also str sometimes accepted for Column. Could you please check these two (and maybe other) signatures for this?

• Argument 1 to "Select" has incompatible type "AbstractSet[Column[Any]]"; expected "Optional[Sequence[Union[ColumnElement[Any], FromClause]]]". Maybe this should be Iterable instead of Sequence?

• Argument 3 to "Select" has incompatible type "Table"; expected "Optional[Sequence[ClauseElement]]". Maybe it accepts a union?

[bryanforbes]

I've addressed the feedback (most were caused by not digging into the underlying code enough), but I'd like to add some tests for what you discovered. Could you give me a few lines of code demonstrating each issue you encountered?

[ilevkivskyi]

Sorry for a delay, I just tried this again. There are still three errors in our codebase:

• One is Argument 1 to "insert" of "TableClause" has incompatible type "Dict[Column[Any], int]"; expected "Optional[Mapping[Union[str, ColumnClause[Any]], Any]]". This because Mapping is invariant in key. You probably need to use a huge union here: Union[None, Mapping[str, Any], Mapping[ColumnClause[Any], Any], Mapping[Union[str, ColumnClause[Any]], Any]]
• Another is Argument "order_by" to "Select" has incompatible type "Sequence[SortOrder]"; expected "Optional[Iterable[Union[int, str, Visitable]]]"
• Finally, there is Argument 3 to "Select" has incompatible type "Table"; expected "Optional[Iterable[Union[str, Selectable]]]" similar to the one appeared with the previous version of the PR (argument 3 is from_obj)

As for tests, the easiest way is to just add dummy tests for affected constructors calls/method calls as I suggested on Gitter.

[ilevkivskyi]

(Also an unrelated thought about speeding things up, I think I can manage to review/try PRs twice as large as this one, so if it is OK for you, I think we can try larger sizes.)

[bryanforbes]

Another is Argument "order_by" to "Select" has incompatible type "Sequence[SortOrder]"; expected "Optional[Iterable[Union[int, str, Visitable]]]"

Is it a bug in typeshed that Sequence doesn't inherit from Iterable?

[bryanforbes]

I just looked. Iterable is a Protocol, so Sequence should match Iterable. What is SortOrder in your code?

[ilevkivskyi]

Oh, sorry, ignore this one. This is a real bug in our codebase found by your new types. :-)

[ilevkivskyi]

@bryanforbes what is the status of this PR? Is any input from my side needed?

[bryanforbes]

@ilevkivskyi No input needed. I just need to put together some tests for the issues you found. I'll try to do that over the weekend or early next week.

[ilevkivskyi]

Great, thanks!

[bryanforbes]

I added a bunch of unit tests testing the issues your found, but I ran into a new issue:

t = table('user', column('id'), column('name'), column('description'), column('articles'))
t.insert({column('name', String): 'foo',
          'description': 'bar'})

mypy infers the type of the argument to insert() as Dict[object, str] instead of Dict[Union[ColumnClause[Any], str], str]. Once that is solved, I think we can merge this.

[ilevkivskyi]

Thanks for an update and for good tests!

I think the problem you encountered is not easy to fix. It looks like there is a mypy bug (or missing feature) here. Basically mypy's type inference sometimes misfires in presence of unions. I would just remove this particular line in your tests, and open an issue on mypy tracker (maybe also try to find a simpler repro), and then I merge this PR.

[bryanforbes]

I commented out the failing invocation. I agree that this is either a bug or a missing feature of mypy. A simple repro is as follows:

d = {'1': 2, 3: 4}
reveal_type(d)  # builtins.dict[builtins.object*, builtins.int*]

I have filed an issue at python/mypy#6079.

[ilevkivskyi]

OK, thanks!