Fix SQLite Aggregation Protocols #12192
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.
|
|
||
| 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. 🤖🎉 |
Fix issues detailed in #12141.