This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author mariocj89
Recipients mariocj89, p-ganssle, vinay.sajip
Date 2019-06-01.09:48:19
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1559382500.2.0.571012311182.issue37117@roundup.psfhosted.org>
In-reply-to
Content
> What do you mean by "it is not documented"? It is documented here:

As it says:

"This function uses a user-configurable function to convert the creation time to a tuple. By default, time.localtime() is used; to change this for a particular formatter instance, set the converter attribute to a function with the same signature as time.localtime() or time.gmtime(). To change it for all formatters, for example if you want all logging times to be shown in GMT, set the converter attribute in the Formatter class." I assumed it means users can set such attribute but not something in the formatter class where people will do formatter.converter(xyz). If the intention was for users to also be able to use the converter attribute then I misunderstood that.

> Obviously, it is also read *somewhere* - otherwise why would you set it?

The implementation of formatTime uses it only at the moment.

> See also questions on Stack Overflow relating to it: https://stackoverflow.com/search?q=%5Bpython%5D+%5Blogging%5D+converter

Through all the questions and answers in stackoverflow they are all setting the converter (which will still work) and some (2 I think) only are reading it when they are also setting it in the class (Example: https://stackoverflow.com/questions/6290739/python-logging-use-milliseconds-in-time-format/6290946#6290946, which by chance is effectively trying to solve the same problem brought up here).

> But the converter is documented as returning a time tuple,

It just says "set the converter attribute to a function with the same signature as time.localtime() or time.gmtime()" there is no reference to what the default value has, it could even be None when user is reading it. Of course, you wrote it hehe, so if you say the intent is for people to use the default formatter that is a different thing :).

>  but that it might break *someone's* code is IMO enough reason not to proceed with it.

I looked hard and could not find any of those in Github (it does not mean that there are none indeed). I would agree on not releasing this as a patch release, but as part of a new minor I would argue that it is fair. From my point of view, it only breaks people depending on the implementation of it and using the formatter in a not common way (at least from what was seen in stackoverflow and github, there is also no cookbook that recommends doing anything like that).

> It's still a one-liner both ways in your example

Yeah but it requires to deeply understand how Formatter is formatting time rather than just using a template.

> Users can always override formatTime in their Formatter subclass to do exactly what they want - it could be a one-time thing that they can then use wherever they want

They need to copy paste this code around on all their scripts/apps or create their own library to include this.

> I don't want to give the impression that I'm against *any* change - I'm not

Don't worry, I can totally understand :), if you still think it is not worth pursuing this feel free to close the issue. It was an idea I thought it was worth pushing but it is totally your call (I won't take it at all personal ^^). Thanks for taking the time to answer!
History
Date User Action Args
2019-06-01 09:48:20mariocj89setrecipients: + mariocj89, vinay.sajip, p-ganssle
2019-06-01 09:48:20mariocj89setmessageid: <1559382500.2.0.571012311182.issue37117@roundup.psfhosted.org>
2019-06-01 09:48:20mariocj89linkissue37117 messages
2019-06-01 09:48:19mariocj89create