Skip to content

Conversation

@DMRobertson
Copy link
Contributor

DBAPI2 specifies that this is a "number" rather than specifically an int, but I think everyone expects this to be an int! It notes that a future version of the API might use None in place of -1 to signal that there's no rowcount value to report. But that would presumably belong in a DBAPI version 3.

Some more justification: CPython's implemention of this is written in C. It uses a long to store the rowcount.

Sorry for not going further here and adding more comprehensive typing!

David Robertson added 2 commits October 11, 2021 13:10
DBAPI2 [specifies](https://www.python.org/dev/peps/pep-0249/#rowcount) that this is a "number" rather than specifically an int. It notes that a future version of the API might use `None` in place of `-1` to signal that there's no rowcount value to report. But that would presumably belong in a DBAPI version 3.
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@srittau srittau merged commit 498f1aa into python:master Oct 11, 2021
DMRobertson pushed a commit to matrix-org/sydent that referenced this pull request Jan 5, 2022
A version of mypy has been released which includes
python/typeshed#6150. (Typeshed is vendored with mypy these days.) We
can now remove the redundant cast.
DMRobertson pushed a commit to matrix-org/sydent that referenced this pull request Jan 5, 2022
* Remove redunant casts

A version of mypy has been released which includes
python/typeshed#6150. (Typeshed is vendored with mypy these days.) We
can now remove the redundant cast.

* Mark logging handler as a generic logging handler.

Since python/typeshed#5681 (vendored in recent mypy releases),
StreamHandler is now generic over its stream. This causes us pain:

> Missing type parameters for generic type "StreamHandler"

And I couldn't find a satisfactory type parameter that worked here.

- a `TimedRotatingFileHandler` instance is a `FileHandler` which is a
`StreamHandler[TextIOWrapper]`. - the instance `StreamHandler()` (which
writes to stdout) is overloaded to be a `StreamHandler[TextIO]`

`handler: StreamHandler[TextIO]` didn't work. I don't fully understand
the difference between the concrete TextIOWrapper and the abstract
TextIO (the former looks to be compatible with the latter?). I think the
StreamHandler type parameter would need to be covariant for this to
work.

In any case, we're not making use of any of the stream or file specific
attributes here. So let's just mark it as a general `Handler`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants