8000 Introduce generic logger type in loggeradapter by hoefling · Pull Request #5954 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Introduce generic logger type in loggeradapter #5954

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 25, 2021
Merged

Introduce generic logger type in loggeradapter #5954

merged 2 commits into from
Aug 25, 2021

Conversation

@hoefling
Copy link
Contributor
@hoefling hoefling commented Aug 24, 2021

The reason for this PR:

import logging

logger = logging.getLogger()
adapter = logging.LoggerAdapter(logger, dict())
reveal_type(adapter.logger)
adapter.logger.getChild("fizz")

has no issues with mypy, but pyright reports

  /tmp/main.py:5:13 - info: Type of "adapter.logger" is "Logger | LoggerAdapter"
  /tmp/main.py:6:16 - error: Cannot access member "getChild" for type "LoggerAdapter"
    Member "getChild" is unknown (reportGeneralTypeIssues)

With the proposed changes, pyright is able to infer the correct type of wrapped logger instance.

@hoefling
introduce generic logger type in loggingadapter
1e7e2c9 
Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
@github-actions

This comment has been minimized.

Sign in to view
@hoefling
work around missing support for recursive typevars
316fa02 
Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
@github-actions
Copy link
Contributor
github-actions bot commented Aug 24, 2021

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

sphinx (https://github.com/sphinx-doc/sphinx.git)
- sphinx/util/logging.py: note: In member "handle" of class "SphinxLoggerAdapter":
- sphinx/util/logging.py:140:9: error: Item "LoggerAdapter" of "Union[Logger, LoggerAdapter]" has no attribute "handle"

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/langserver.py:129: error: Missing type parameters for generic type "LoggerAdapter"  [type-arg]
+ porcupine/plugins/langserver.py:143: error: Missing type parameters for generic type "LoggerAdapter"  [type-arg]
+ porcupine/plugins/langserver.py:323: error: Missing type parameters for generic type "LoggerAdapter"  [type-arg]
+ porcupine/plugins/langserver.py:724: error: Missing type parameters for generic type "LoggerAdapter"  [type-arg]

@srittau
Copy link
Collaborator
srittau commented Aug 24, 2021

Cc @Akuli as someone affected by this change.

Akuli
Akuli approved these changes Aug 25, 2021
View reviewed changes
Copy link
Collaborator
@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

I have some functions that take an argument of type logging.LoggerAdapter, and I just have to change those to logging.LoggerAdapter[logging.Logger].

@JelleZijlstra JelleZijlstra merged commit a74624d into python:master Aug 25, 2021
@hoefling
Copy link
Contributor Author
hoefling commented Aug 25, 2021 •
edited
Loading

@Akuli @srittau is it an issue that one has to distinct between type checking and runtime now? I mean, LoggerAdapter is not subscriptable, so the type has to be either quoted

x: "logging.LoggerAdapter[logging.Logger]"  = ...

or aliased via e.g.

if TYPE_CHECKING:
    import logging
    LoggerAdapter = logging.LoggerAdapter[logging.Logger]
else:
    from logging import LoggerAdapter

x: LoggerAdapter = ...

@srittau
Copy link
Collaborator
srittau commented Aug 25, 2021

That's a general issue, but temporary. I would recommend to use from __future__ import annotations if possible or quoting of not possible.

@hoefling hoefling deleted the generic-logger-adapter branch September 2, 2021 07:37
@onursatici
Copy link
onursatici commented May 1, 2022

@srittau @hoefling What would be the recommendation for inheritance? From what I understand importing annotations or quoting would not be as useful here

import logging

class Adapter(logging.LoggerAdapter[logging.Logger]):
    pass

As LoggerAdapter is not subscriptable, this class will fail to initialise

@Akuli
Copy link
Collaborator
Akuli commented May 1, 2022

This is a common problem, and unfortunately it's annoying to deal with. This works, but it's messy and feels like a hack:

import logging
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    _LoggerAdapter = logging.LoggerAdapter[logging.Logger]
else:
    _LoggerAdapter = logging.LoggerAdapter

class MyAdapter(_LoggerAdapter):
    pass

Things that would help, but don't exist yet:

  • Default values for generics (Defaults for Generics? mypy#4236)
  • Pull request to CPython that makes logging.LoggerAdapter generic at runtime (similar PRs have been accepted in the past)

@hauntsaninja
Copy link
Collaborator
hauntsaninja commented May 1, 2022

Linking the relevant section of mypy docs, in case it helps people: https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-classes-that-are-generic-in-stubs-but-not-at-runtime

@AlexWaygood AlexWaygood mentioned this pull request May 1, 2022
Closed
@AlexWaygood
Copy link
Member
AlexWaygood commented May 1, 2022 •
edited
Loading

I've opened python/cpython#92129 to add __class_getitem__ to the class in 3.11+.

(Update: merged!)

@AlexWaygood AlexWaygood mentioned this pull request May 17, 2022
Closed
@AlexWaygood AlexWaygood mentioned this pull request Oct 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srittau srittau srittau approved these changes

@Akuli Akuli Akuli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hoefling @srittau @onursatici @Akuli @hauntsaninja @AlexWaygood @JelleZijlstra
0