python / typeshed

Collection of library stubs for Python, with static types
Other
4.15k stars 1.68k forks source link

incomplete type declaration at `sqlite3.Connection.create_aggregate()` #12141

Open kunom opened 2 weeks ago

kunom commented 2 weeks ago

Bug report

Bug description:

The create_aggregate documentation does not specify a result type of the aggregation and even allows for multiple aggregating values per row (n_arg parameter).

However, the type declaration at dbapi2.pyi does not reflect that (comments added by me):

def create_aggregate(self, name: str, n_arg: int, aggregate_class: Callable[[], _AggregateProtocol]) -> None: ...

class _AggregateProtocol(Protocol):
    def step(self, value: int, /) -> object: ...  # <-- just one aggregating value of type int
    def finalize(self) -> int: ...   # <-- result has to be int

I think that this is not correct. At least, I have working code that aggregates multiple strings into a combined one.

Also, it is in stark contrast to other, way more generic, type declarations at the same place (comments added by me):

class _AnyParamWindowAggregateClass(Protocol):
    def step(self, *args: Any) -> object: ... # <-- multiple aggregating values of any type allowed
    def inverse(self, *args: Any) -> object: ...
    def value(self) -> _SqliteData: ...
    def finalize(self) -> _SqliteData: ... # <-- result can be many more types

_SqliteData: TypeAlias = str | ReadableBuffer | int | float | None

CPython versions tested on:

3.12

Operating systems tested on:

Windows

Eclips4 commented 2 weeks ago

Hello! This repository is intended as a CPython bug tracker, your problem is actually related to stubs which is not part of CPython; It's a part of typeshed. I can't transfer this issue to the typeshed repo, @AlexWaygood maybe you can? 🙂

sobolevn commented 2 weeks ago

This is indeed a typeshed problem. CPython documents that finalize() returns:

finalize(): Return the final result of the aggregate as a type natively supported by SQLite.

Looks like this protocol should be generic.

kunom commented 2 weeks ago

Just to summarize my request: