Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make logging.StreamHandler generic over stream #5681

Merged
merged 1 commit into from Jun 23, 2021

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Jun 23, 2021

Closes: #5680

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pip (https://github.com/pypa/pip.git)
+ src/pip/_internal/utils/logging.py:152: error: Missing type parameters for generic type "StreamHandler"

pytest (https://github.com/pytest-dev/pytest.git)
+ src/_pytest/main.py:505: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)
- src/_pytest/main.py:505: error: Returning Any from function declared to return "Path"  [no-any-return]
- src/_pytest/python.py:411: error: Need type annotation for "dicts"  [var-annotated]
- src/_pytest/logging.py:619: error: Argument 1 to "setStream" of "FileHandler" has incompatible type "TextIO"; expected "TextIOWrapper"  [arg-type]
+ src/_pytest/logging.py:318: error: Missing type parameters for generic type "StreamHandler"  [type-arg]
+ src/_pytest/logging.py:754: error: Missing type parameters for generic type "StreamHandler"  [type-arg]
- src/_pytest/cacheprovider.py:143: error: Returning Any from function declared to return "Path"  [no-any-return]
- src/_pytest/cacheprovider.py:153: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)

@srittau
Copy link
Collaborator Author

srittau commented Jun 23, 2021

Looking at primer output: A few cases where the type parameter must be added (another case where generic defaults would be useful). The rest don't seem related to this PR.

@Akuli Akuli merged commit dd44164 into python:master Jun 23, 2021
@srittau srittau deleted the logging-stream branch June 23, 2021 14:17
DMRobertson pushed a commit to matrix-org/sydent that referenced this pull request Jan 5, 2022
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`.
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.

logging.StreamHandler.stream is sys.stderr by default ; "SupportsWrite['str']" is too narrow
2 participants