Skip to content

Introduce generic logger type in loggeradapter#5954

Merged
JelleZijlstra merged 2 commits into
python:masterfrom
hoefling:generic-logger-adapter
Aug 25, 2021
Merged

Introduce generic logger type in loggeradapter#5954
JelleZijlstra merged 2 commits into
python:masterfrom
hoefling:generic-logger-adapter

Conversation

@hoefling

Copy link
Copy Markdown
Contributor

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.

Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
@github-actions

This comment has been minimized.

Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

srittau commented Aug 24, 2021

Copy link
Copy Markdown
Collaborator

Cc @Akuli as someone affected by this change.

@Akuli Akuli left a comment

Copy link
Copy Markdown
Collaborator

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

hoefling commented Aug 25, 2021

Copy link
Copy Markdown
Contributor Author

@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

srittau commented Aug 25, 2021

Copy link
Copy Markdown
Collaborator

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
Copy Markdown

@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

Akuli commented May 1, 2022

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

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 commented May 1, 2022

Copy link
Copy Markdown
Member

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

(Update: merged!)

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.

7 participants