Skip to content

Conversation

DanielNoord
Copy link
Contributor

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.

@DanielNoord
Copy link
Contributor Author

@elprans Could you retrigger the CI so this can get merged? The failure is a flake and I don't have retriggering rights.

@jmehnle
Copy link

jmehnle commented Jan 3, 2025

@elprans: ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants