rogerbinns / apsw

Another Python SQLite wrapper
https://rogerbinns.github.io/apsw/
Other
715 stars 96 forks source link

Cursor.executemany should be annotated as accepting Iterable[Bindings] instead of Sequence[Bindings] #508

Closed spikethehobbitmage closed 5 months ago

spikethehobbitmage commented 5 months ago

Cursor.executemany is currently annotated as taking Sequence[Bindings] but it actually accepts the more general Iterable[Bindings]. This causes working code that uses generators and iterators to fail type checking.

Correcting the annotation will not adversely affect existing code since an Iterable parameter will still accept a Sequence value.

For example, the following example script runs correctly:

test.py

#!/usr/bin/python3

from typing import Iterable, Sequence
import apsw

def makeRows() -> Iterable['apsw.Bindings']:
    for row in [
        (1, 2),
        (3, None),
        (5, 6)]:
        yield row

connection = apsw.Connection(':memory:')

connection.cursor().execute("""\
CREATE TABLE "table1" (
    "columnA" INTEGER NOT NULL PRIMARY KEY,
    "columnB" INTEGER
);""")

connection.cursor().executemany("""INSERT INTO "table1"("columnA", "columnB") VALUES (?, ?);""", makeRows())

but calling "mypy test.py" returns the following error:

test.py:22: error: Argument 2 to "executemany" of "Cursor" has incompatible type "Iterable[Sequence[int | float | bytes | str | None | zeroblob] | Mapping[str, int | float | bytes | str | None | zeroblob]]"; expected "Sequence[Sequence[int | float | bytes | str | None | zeroblob] | Mapping[str, int | float | bytes | str | None | zeroblob]]"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

Edit:fixed formatting

rogerbinns commented 5 months ago

Thanks for finding this. The underlying C api used is PySequence which is how I got the type annotation, but Iterable is more correct. I do wish there was a list of the C apis and their corresponding type annotation.

The following commit fixes all usages of Sequence, and will be in the next release.