Typed queries 2 (reduced)

[tiangolo]

First, thanks for making these stubs and plugin! It works great. I'm using it with Pyright and that gives me autocompletion in VS Code for SQLAlchemy stuff, that never worked before. :sparkles:

---

This PR includes a reduced version of the changes by @rafales in #81 .

I removed the plugin method hook and its tests to reduce the scope of the PR, to make it easier to manage and merge, as just the type annotations are a great improvement.

I applied @ilevkivskyi 's code review from that PR here as well.

---

There were some unresolved questions/discussions in that PR, so I removed any changes related to those sections.

My objective is just to reduce the scope of the changes to the minimum that would be easily acceptable and postpone any additional improvement for future PRs.

[tiangolo]

About the functionality provided by the plugin hook, I was thinking that maybe a simpler approach (for a future PR) would be to add several overloads to the query method, similar to #121 .

Let's say 8 or 10 overloads, with combinations for querying from 1 to 10 entities, and setting the right return type, e.g. something like Query[Tuple[_T1, _T2]] for 2 entities, etc.

That would allow supporting a predefined number of arguments for query. And as it wouldn't depend on a mypy plugin, it would probably also work with other tools (e.g. Pyright).

Anyway, I would leave that for a future PR after this, to keep the scope limited.

[ilevkivskyi]

Let's say 8 or 10 overloads, with combinations for querying from 1 to 10 entities, and setting the right return type, e.g. something like Query[Tuple[_T1, _T2]] for 2 entities, etc.

Since at every position you can have either an entity or a column, the number of overloads grows exponentially. But maybe this can be a possible temporary hack to have a bit better types until a plugin is implemented.

[tiangolo]

Thanks for the review @ilevkivskyi !

Yep, I signed the CLA it right before opening the PR, following the link from the README.

Is there anything else I should do about the CLA?

---

the number of overloads grows exponentially

Yeah, I just realized it :sweat_smile:

Right now I'm playing locally with something like:

    @overload
    def query(self, entity: Union[Type[_T1], Column[_T1]], **kwargs) -> Query[_T1]: ...
    @overload
    def query(self, entity1: Union[Type[_T1], Column[_T1]], entity2: Union[Type[_T2], Column[_T2]], **kwargs) -> Query[Tuple[_T1, _T2]]: ...

But I'm not sure what other types of entities could go there, and if they are all Generics so that that trick is possible. :thinking:

[ilevkivskyi]

Right now I'm playing locally with something like

Using a union is a clever solution, but even with it there is an issue of returning a plain tuple for people who use patterns like this:

for row in session.query(User.id, User.name):
    do(row.id, row.name)  # mypy will complain that tuple doesn't have attributes "id" and "name"

So it looks like we can't avoid a plugin here.

[tiangolo]

Thanks for merging it! :tada:

So it looks like we can't avoid a plugin here.

Yep, agreed.

Although maybe it's possible to improve the version with just types a little bit, even if it's a bit hackish, anyway, I'll play with it a bit more as soon as I have the chance.

---

Thanks handling this (review, merge) so quickly! :bow: :rocket:

[delaaxe]

This is great! How can we start using this? Can we publish a new version with this change?

[NotarySigningAgent]

idea drafting, i will delete if preferred?

On Mon, Jun 15, 2020 at 1:03 AM delaaxe notifications@github.com wrote:

This is great! How can we start using this? Can we publish a new version with this change?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.