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

Introduce generic logger type in loggeradapter #5954

Merged
merged 2 commits into from
Aug 25, 2021

Conversation

hoefling
Copy link
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
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
Copy link
Collaborator

srittau commented Aug 24, 2021

Cc @Akuli as someone affected by this change.

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

@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

@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

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
Copy link
Member

AlexWaygood commented May 1, 2022

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