Additional return types for psycopg2 connections #8528
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
I'm not sure of the best approach to solving the mypy errors in CI. The cases where it is reporting that an override's return types are incompatible with the super return types are how it is actually implemented in the underlying library's code (i.e. the stubs I've added merely reflect the reality of the code). Thoughts? |
|
Stubs should reflect what the library actually does. We use |
| def cursor(self, *args, **kwargs): ... | ||
| @overload | ||
| def cursor(self, name: str | bytes | None = ..., *, withhold: bool = ..., scrollable: bool | None = ...) -> DictCursor: ... | ||
| @overload | ||
| def cursor( | ||
| self, | ||
| name: str | bytes | None = ..., | ||
| *, | ||
| cursor_factory: Callable[..., _T_cur], | ||
| withhold: bool = ..., | ||
| scrollable: bool | None = ..., | ||
| ) -> _T_cur: ... | ||
| @overload | ||
| def cursor( | ||
| self, name: str | bytes | None, cursor_factory: Callable[..., _T_cur], withhold: bool = ..., scrollable: bool | None = ... | ||
| ) -> _T_cur: ... |
There was a problem hiding this comment.
These overloads are consistent with the base class's definitions of the cursor method except that the default cursor_factory case has been updated to return DictCursor. This comment also applies to RealDictConnection and NamedTupleConnection.
| def fetchone(self) -> DictRow | None: ... # type: ignore[override] | ||
| def fetchmany(self, size: int | None = ...) -> list[DictRow]: ... # type: ignore[override] | ||
| def fetchall(self) -> list[DictRow]: ... # type: ignore[override] | ||
| def __next__(self) -> DictRow: ... # type: ignore[override] |
There was a problem hiding this comment.
The return types aren't compatible with the base cursor class's definition, but that is consistent with the reality of the implementation. This comment also applies to the other modified cursor type defs.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
All right. I have made several changes, including adding |
stubs/psycopg2/psycopg2/extras.pyi
Outdated
| class DictCursorBase(_cursor): | ||
| row_factory: Any | ||
| def __init__(self, *args, **kwargs) -> None: ... | ||
| def fetchone(self) -> tuple[Any, ...] | None: ... | ||
| def fetchmany(self, size: int | None = ...) -> list[tuple[Any, ...]]: ... | ||
| def fetchall(self) -> list[tuple[Any, ...]]: ... | ||
| def __iter__(self): ... | ||
| def __next__(self) -> tuple[Any, ...]: ... |
There was a problem hiding this comment.
What is the reason for this change? Both __iter__ and __next__ are inherited from cursor, so we don't need them here.
There was a problem hiding this comment.
Indeed, you're right. in fact fetchone, fetchmany, fetchall and row_factory are all superfluous too. I'll clean them up.
|
|
||
| class DictCursor(DictCursorBase): | ||
| def __init__(self, *args, **kwargs) -> None: ... | ||
| index: Any | ||
| def execute(self, query, vars: Any | None = ...): ... | ||
| def callproc(self, procname, vars: Any | None = ...): ... | ||
| def fetchone(self) -> DictRow | None: ... # type: ignore[override] |
There was a problem hiding this comment.
An alternative could be to make the base class generic. But that may cause problems until we have TypeVar defaults.
| def fetchmany(self, size: int | None = ...) -> list[tuple[Any, ...]]: ... | ||
| def fetchall(self) -> list[tuple[Any, ...]]: ... | ||
| def __iter__(self): ... | ||
| def fetchone(self) -> NamedTuple | None: ... # type: ignore[override] |
There was a problem hiding this comment.
Returning typing.NamedTuple is a little weird, but it apparently works with both pyright and mypy, so I suppose it's fine.
There was a problem hiding this comment.
Yes, I agree. But at least it offers a hint to the caller that they can use either list indexing or access the fields with dot-notation.
This comment has been minimized.
This comment has been minimized.
|
One new commit for review. |
DB connection and cursor methods that deal with `dict`-style results now indicate the expected return types as implemented.
DB connection and cursor methods that deal with `namedtuple` results now indicate the expected return types as implemented.
As required by flake8, the `Iterator` referenced in psycopg2's `extras` module has been switched to the `collections.abc.Iterator[T]` variant.
The previous return type of `Self` is wrong; `cursor`'s `__iter__` method returns an `Iterator`.
The previous attempt to fix the return type of `cursor`'s (and subclasses) `__iter__` method was misguided; they should indeed return `Self`. It's the `__next__` methods that had to be updated to return the correct record/row instance type.
Provides full method signatures for the `cursor` methods of the `DictConnection`, `RealDictConnection` and `NamedTupleConnection` types. Notably this includes a default value for `cursor_factory` in each case, while preserving the option to override the parameter manually.
The return types of the `fetch*` and `__next__` methods of `DictCursor`, `RealDictCursor` and `NamedTupleCursor` are incompatible with the base `cursor` class's corresponding methods' return types. However, this does accurately reflect reality, so ignore the mypy errors in those cases.
As required by flake8, the `Callable` referenced in psycopg2's `extras` module has been switched to the `Callable` variant.
Several members in the `DictCursorBase` type definition were entirely unnecessary, so they have been removed. In addition, adds a type to the `size` param of the `cursor`'s `fetchmany` method.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Adds correct return types for DB connection and cursor methods that deal with
namedtupleanddict-like cursors. Currently the types are either incomplete (e.g. the connections'cursormethods do not specify return types) or wrong (e.g.DictCursor'sfetch*methods should not returntuples, which is currently inherited fromDictCursorBase).