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

Logging getLogger Documentation Incomplete #127313

Open
Wikilicious opened this issue Nov 27, 2024 · 7 comments
Open

Logging getLogger Documentation Incomplete #127313

Wikilicious opened this issue Nov 27, 2024 · 7 comments
Labels
docs Documentation in the Doc dir

Comments

@Wikilicious
Copy link

Documentation

The latest logging.getLogger documentation is incomplete/inaccurate.

Specifically, "If no name is specified, return the root logger.".

To more accurately reflect the code...
Change it to "If name is Falsy or 'root', return the root logger."

logging.getLogger https://github.com/python/cpython/blob/3.13/Lib/logging/__init__.py#L2128

def getLogger(name=None):
    """
    Return a logger with the specified name, creating it if necessary.

    If no name is specified, return the root logger.
    """
    if not name or isinstance(name, str) and name == root.name:
        return root
    return Logger.manager.getLogger(name)

Examples that will return root logger:

logger = logging.getLogger()
logger = logging.getLogger("")
logger = logging.getLogger("root")
logger = logging.getLogger(None)
logger = logging.getLogger(False)
logger = logging.getLogger(0)  
@Wikilicious Wikilicious added the docs Documentation in the Doc dir label Nov 27, 2024
@picnixz
Copy link
Contributor

picnixz commented Nov 27, 2024

I don't think this is needed. We don't necessarily want those side effects to be documented. The root logger should not necessarily be named "root", it may change in the future and we may want to make that change invisible.

Semantically speaking, it does not make sense to pass a falsey value to the function even if it's accepted at runtime. So I don't think we will document that.

What we want to document is "the root logger can be retrieved via logging.getLogger()" and that's essentially the only way we want for the public API.

cc @vsajip

@picnixz picnixz added the pending The issue will be closed if no feedback is provided label Nov 27, 2024
@Wikilicious
Copy link
Author

Perhaps not needed... but, useful in guiding future examples.

e.g.
https://docs.python.org/3/howto/logging-cookbook.html#logging-to-multiple-destinations

# add the handler to the root logger
logging.getLogger('').addHandler(console)

https://docs.python.org/3/howto/logging-cookbook.html#dealing-with-handlers-that-block
root = logging.getLogger() note var=root
https://docs.python.org/3/howto/logging-cookbook.html#imparting-contextual-information-in-handlers
logger = logging.getLogger() note var=logger

@picnixz
Copy link
Contributor

picnixz commented Nov 27, 2024

I think we can unify the examples then and avoid using '' as the name when possible.

@ZeroIntensity
Copy link
Member

logging.getLogger("root") is possibly worth documenting, but the "falsy returning root" behavior is an implementation detail, and I don't think it should be documented. It's not guaranteed that passing an integer or something like that will work in the future.

@picnixz
Copy link
Contributor

picnixz commented Nov 28, 2024

I'm not fond of documenting "root" because the runtime code checks for root.name which may change in the future or may be changed externally (IIRC you could technically get the root logger and directly change its name via root.name = ...).

@Wikilicious
Copy link
Author

I like the combination of treating the falsy part as an implementation detail (@ZeroIntensity) and removing references to getLogger('') in the logging-cookbook/docs (@picnixz).

The name "root" is checked at runtime; however, the class RootLogger is hardcoded with name="root" and no exposed method to change its name.
My thought process is the docs insinuates None is the only way to get the root logger. In my mind... I'd assume all strings would propagate to the root logger... but, the name "root" is treated as a reserved name... it is the root logger. I feel that's a detail worth documenting.

@picnixz
Copy link
Contributor

picnixz commented Nov 29, 2024

and no exposed method to change its name.

You're right but the attribute is still without an underscore and people could think it's public so there might be code in the wild that changes the name (even if it's incorrect). If, in the future, we were to change the name, we don't necessarily want to expose the fact that "root" is the name. So, instead, I really think the root logger should be retrieved via logger.getLogger(). But, if @vsajip thinks that "root" should be a reserved name, then we I'm not against documenting it.

@picnixz picnixz removed the pending The issue will be closed if no feedback is provided label Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
Status: Todo
Development

No branches or pull requests

3 participants