Skip to content

Logger formatter class initialization state clarification #127805

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

Closed
jlynchMicron opened this issue Dec 10, 2024 · 6 comments · Fixed by #127850
Closed

Logger formatter class initialization state clarification #127805

jlynchMicron opened this issue Dec 10, 2024 · 6 comments · Fixed by #127850
Labels
docs Documentation in the Doc dir

Comments

@jlynchMicron
Copy link

jlynchMicron commented Dec 10, 2024

Documentation

https://docs.python.org/3/library/logging.html#logging.Handler.setFormatter

When providing a logger handler with a formatter class, there is no documentation stating if that formatter class needs to be initialized before passing it as an argument. Through trial and error, I found that it needs to be initialized first. Please either clarify the documentation or have underlying code check to see if the formatter is initialized yet.

Problemed formatter configuration:

handler.setFormatter(xml_log_formatter)
logging.info('test_msg')

Error:
image

Fixed formatter configuration:

handler.setFormatter(xml_log_formatter())
logging.info('test_msg')

Purposed code check (from a custom logging module):

#Instantiate formatter class (if needed)
if isinstance(cls.log_formatter, type):
    cls.log_formatter = cls.log_formatter()

Linked PRs

@jlynchMicron jlynchMicron added the docs Documentation in the Doc dir label Dec 10, 2024
@Uvi-12
Copy link

Uvi-12 commented Dec 11, 2024

@jlynchMicron I would like to work on this, I wanted to ask how necessary the proposed change to the setFormatter method is. Would allowing it to accept both formatter instances and classes have a significant impact on the usability or functionality of the logging module? The current behavior works if users follow the documentation, though it could be clearer.

Is this change something that is critical to address, or would clarifying the documentation alone be sufficient?

@jlynchMicron
Copy link
Author

Hi @Uvi-12, I believe clarification in the documentation would totally be sufficient. The error message generated when using an un-initialized formatter class is not very clear to a more novice programmer, especially when their formatter.format function does have a "record" positional argument.

@Uvi-12
Copy link

Uvi-12 commented Dec 11, 2024

Hi @Uvi-12, I believe clarification in the documentation would totally be sufficient. The error message generated when using an un-initialized formatter class is not very clear to a more novice programmer, especially when their formatter.format function does have a "record" positional argument.

Thank you for the clarification! I understand that the error message can be confusing, especially for novice users. Given that, I’ll go ahead and work on improving the documentation to make it clearer and help prevent misunderstandings related to un-initialized formatter classes.

Let me know if there are any additional points I should consider while working on this!

@jlynchMicron
Copy link
Author

I think that should be it, thanks!

@Uvi-12
Copy link

Uvi-12 commented Dec 12, 2024

@jlynchMicron Please check PR #127850 and let me know if it solves the issue.

@jlynchMicron
Copy link
Author

@Uvi-12 looks good to me!

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 21, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 21, 2025
terryjreedy pushed a commit that referenced this issue Feb 24, 2025
…H-127850) (#130393)

gh-127805: Clarify Formatter initialization in logging.rst. (GH-127850)
(cherry picked from commit 5d66c55)

Co-authored-by: UV <[email protected]>
hugovk added a commit that referenced this issue Feb 24, 2025
…H-127850) (#130392)

(cherry picked from commit 5d66c55)

Co-authored-by: UV <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
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

Successfully merging a pull request may close this issue.

2 participants