gh-127805: Clarify Formatter initialization in logging.rst

[Uvi-12]

Clarified that the fmt argument in Handler.setFormatter must be an initialized Formatter instance in the logging.rst documentation.

Issue:
Resolves #127805

• Issue: Logger formatter class initialization state clarification  #127805

---

📚 Documentation preview 📚: https://cpython-previews--127850.org.readthedocs.build/en/127850/library/logging.html#logging.Handler.setFormatter

[ghost]

All commit authors signed the Contributor License Agreement.

[ZeroIntensity]

Thanks! LGTM.

cc @picnixz for a second look.

[Uvi-12]

Thank you @picnixz @ZeroIntensity for reviewing PR! If everything looks good, please proceed with merging it.

[ZeroIntensity]

Neither of us have merge permissions, we're triagers (though Bénédikt might soon)! You'll have to wait for a core dev to get to this.

[Uvi-12]

Neither of us have merge permissions, we're triagers (though Bénédikt might soon)! You'll have to wait for a core dev to get to this.

Thank you for clarifying, @ZeroIntensity! I apologize for the oversight, as I’m still new to Python and the contribution process.

[vsajip]

The wording of the change might imply that the instance has to be one of the Formatter class only, and not a subclass. In fact I wonder that the person who raised the issue thought that it was being suggested that you pass a class to a handler in setFormatter - passing a class to an API is a rather less common case than passing an instance, generally, in Python, right?

[picnixz]

Well... even if it's an instance of a subclass, isinstance(formatter, Formatter) would still work right? So I think the formulation is fine. But maybe I'm biased?

[Uvi-12]

Thank you, @vsajip and @picnixz, for the feedback!

@jlynchMicron, who created the issue, reviewed the PR and confirmed that the changes are correct in their comment.

The current wording is meant to make it clear that the formatter argument must be an initialized instance of Formatter or a subclass. I agree that it might sound like only Formatter is allowed, which could be confusing.

Would it help if I updated the wording to say something like "an initialized instance of Formatter or one of its subclasses"? Let me know, and I can make the changes to avoid any misunderstanding.

[vsajip]

Well... even if it's an instance of a subclass, isinstance(formatter, Formatter) would still work right?

Yes, but we're talking about just a documentation change. For example, I think the existing text (mentioning Formatter) is already reasonably understandable to mean a Formatter instance rather than the Formatter class, but if this is likely to be a point of confusion for some people, we might as well spell it out in a bit more detail, as suggested ...

Would it help if I updated the wording to say something like "an initialized instance of Formatter or one of its subclasses"?

Yes, I think it would.

[picnixz]

Ok, I see your point. To be consistent, should we also update the setFilter docs just below?

[vsajip]

should we also update the setFilter docs just below?

I guess we could. But let's think about this - do we really want to go through the documentation checking for all instances of where a class is mentioned, meaning that an instance is required, and updating all cases in case they might be misunderstood?

The usual way to interpret when you say "pass in a Formatter" is to infer that an instance is to be passed, and and not the Formatter class itself - so I don't actually believe the issue is valid and this PR is needed, but I don't want to appear inflexible if it's just a tiny change to the documentation.

But then someone might say "it's not really clear that Logger.addHandler takes an instance of a handler, I thought it meant a handler class" and there you go again. I'd like to avoid that - for example, this API has been pretty much the same for twenty years, and in all that time, nobody else has seemingly gotten confused into thinking that a class is to be passed rather than an instance - at least, not enough to raise an issue about it.

Sometimes the problem is the documentation, but sometimes it's reader comprehension (bearing in mind different people have different mother tongues, different levels of experience as developers, etc). Whatever the documentation says, there will be some readers who can misinterpret it - IMO one can only go so far in accommodating all such cases.

[Uvi-12]

Sometimes the problem is the documentation, but sometimes it's reader comprehension (bearing in mind different people have different mother tongues, different levels of experience as developers, etc). Whatever the documentation says, there will be some readers who can misinterpret it - IMO one can only go so far in accommodating all such cases.

Yes, I agree with the points you raised. I think it's important to keep things clear from our side, but also acknowledge that there's a limit to how much we can accommodate every potential misinterpretation. That said, if you have any suggestions for additional documentation or clarifications that could make the intent absolutely clear, we can certainly consider them. Otherwise, we can close the issue if we believe it's a matter of reader comprehension.

[Uvi-12]

But then someone might say "it's not really clear that Logger.addHandler takes an instance of a handler, I thought it meant a handler class" and there you go again. I'd like to avoid that - for example, this API has been pretty much the same for twenty years, and in all that time, nobody else has seemingly gotten confused into thinking that a class is to be passed rather than an instance - at least, not enough to raise an issue about it.

While I agree with the point that the usual interpretation of "pass in a Formatter" implies an instance rather than the class itself, the fact that this issue was raised indicates some level of confusion among readers, which suggests that clarifying it in the documentation could be helpful. However, we don't need to go through every API documentation and update it proactively; if users face issues, they can raise them, and we can address them like this issue.

@vsajip Could you please let me know your thoughts on this or what steps you'd recommend to take next?

[jlynchMicron]

Just to chime in here as the person that originally raised the issue; maybe it's just me, but I have written a lot of funky python code that has functions that takes in uninitialized classes as function arguments. I think this sort of thing happens more in the data conditioning and database ORM space where dataclasses are used as formats/templates for data conditioning (which is kinda what a Formatter class does).

So that being said, I could be in the minority when reading documentation that says pass in a class and not immediately think to instantiate that class first. The error generated from using the format function of an un-instantiated Formatter is sort of non-intuitive until you realize that the "record" argument is being passed in as "self".

This issue was an attempt to possibly address that confusion for others.

[Uvi-12]

@jlynchMicron @vsajip Taking everyone’s feedback into account, I understand that the API has been stable for years and that the usual interpretation of “pass in a Formatter” inherently implies an instance rather than the class itself. However, given that the issue was raised—alongside @jlynchMicron’s experience—it does suggest that there might be room for confusion for some readers. The proposed wording update to "an initialized instance of Formatter or one of its subclasses" aims to clarify the intent without adding unnecessary verbosity.

That said, I’d like to confirm: would you recommend proceeding with this wording change for clarity, extending it to setFilter for consistency as suggested by @picnixz, or do you think the issue and PR could be closed altogether, considering your points on reader comprehension and the longstanding clarity of the API? I’ll proceed based on your final recommendation.

[picnixz]

The confusion might arise from the fact that we refer to "Formatter" which leads to the Formatter class by saying "Sets the Formatter [...]". How about rephrasing it as

Sets the formatter object for this handler to *fmt*. The *fmt* argument
must be a :class:`Formatter` instance or ``None``.

I feel that mentioning subclasses or "initialized" would be overly verbose. While I see the distinction with "initialized" (through __init__) and newly created (with __new__ only), I don't think we expect a non-initialized object in general.

Concerning setFilter, maybe consistency is not needed. The reason is that we may have multiple filters and a None filter is not expected. However, a None formatter is allowed. So it should be mentioned (currently it's not even mentioned). So the
consistency question is (on my side) dropped.

An alternative to changing the wording is to refer to some examples, without changing the current wording. From there, it would become clearer what we expect right?

[Uvi-12]

The confusion might arise from the fact that we refer to "Formatter" which leads to the Formatter class by saying "Sets the Formatter [...]". How about rephrasing it as

Sets the formatter object for this handler to *fmt*. The *fmt* argument
must be a :class:`Formatter` instance or ``None``.

Concerning setFilter, maybe consistency is not needed. The reason is that we may have multiple filters and a None filter is not expected. However, a None formatter is allowed. So it should be mentioned (currently it's not even mentioned). So the consistency question is (on my side) dropped.

I too think of it as the correct approach. Shall I update the PR with this change?

[vsajip]

Sorry for slow response - I have had a few other things going on. Will look at this soon, thanks for your patience.

[vsajip]

Shall I update the PR with this change?

Yes, please @Uvi-12 , although IMO you could leave out "object" as that should be understood:

Sets the formatter for this handler to fmt

Seems clear enough. And thank you for your patience!

[vsajip]

As per my last comment.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[vsajip]

There's an odd problem where the PR is approved but not mergeable because of some unexplained failure ... will close and re-open the PR to see if that sorts things out.

[miss-islington-app]

Thanks @Uvi-12 for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

[bedevere-app]

GH-130392 is a backport of this pull request to the 3.13 branch.

[bedevere-app]

GH-130393 is a backport of this pull request to the 3.12 branch.