Fix SQLite Aggregation Protocols#12192
Uh oh!
There was an error while loading. Please reload this page.
Conversation
max-muoto changed the title 
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| deffinalize(self) ->_SQLType: ... | ||
| class_WindowAggregateClass(Protocol): | ||
| step: Callable[..., object] |
There was a problem hiding this comment.
From testing things out, it doesn't seem this protocol really works as intended in either MyPy or Pyright. Unless you actually were annotating a lambda perhaps. Due to this, I went ahead and removed it.
Some examples of how it might not work as you would expect:
There was a problem hiding this comment.
This has been part of the initial commit in #7625, while the other protocols already used a function. Maybe @JelleZijlstra remembers why we used an attribute instead of a function here?
There was a problem hiding this comment.
I don't really remember what I was thinking when I wrote that code, but the annotations proposed in this PR mean that protocol implementations must take *args. I am not familiar with how these things are used, but I'd expect concrete implementations to only accept a fixed number of parameters. Maybe that's why I chose to use Callable[....
There was a problem hiding this comment.
I tried messing around with TypeVarTuples to do that, but had some issues there as well, there might not be a great way, but I'll see if I can figure something out.
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
alex commented Aug 17, 2025
Is there a reason this was closed? |
4 participants
Fix issues detailed in #12141.