Add typing to `Pool.acquire` #1209

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

Jump to bottom

Open

DanielNoord wants to merge 1 commit into MagicStack:master

from DanielNoord:type-acquire

Open

Add typing to `Pool.acquire` #1209

DanielNoord wants to merge 1 commit into MagicStack:master from DanielNoord:type-acquire

+14 −9

Conversation

DanielNoord

Copy link

Contributor

@DanielNoord DanielNoord commented Nov 4, 2024

Now that #1197 landed I could take the patch I suggested in #578 and add some more minor changes.

Therefore, this closes #578 and closes #521

In the long run, this will probably cause issues. Proxies can't really be expressed that well on the type level, so we will probably get a lot of "Unknown attribute" errors on everything that is using whatever comes out of acquire().
However, I would personally advise not to worry about this too much. Let's first add typing to those places where we know it will work in incremental steps and consider larger issues when we have some more experience with adding the types.

We'll likely also run into issues identified in #577 but as long as we don't add any typing that is incorrect this can still be solved.

There is (obviously) one behaviour change with the assert, but as I read the code (and tests) I don't think that should be an actual change in behaviour.