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 decimal point should come from locale #74141

Closed
smontanaro opened this issue Mar 31, 2017 · 11 comments
Closed

logging decimal point should come from locale #74141

smontanaro opened this issue Mar 31, 2017 · 11 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@smontanaro
Copy link
Contributor

BPO 29955
Nosy @smontanaro, @rhettinger, @vsajip, @pitrou, @zhangyangyu
PRs
  • bpo-29955: get the decimal point from the active locale #931
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2017-04-06.18:47:28.727>
    created_at = <Date 2017-03-31.17:18:07.585>
    labels = ['3.7', 'invalid', 'type-bug', 'library']
    title = 'logging decimal point should come from locale'
    updated_at = <Date 2017-04-06.18:47:28.725>
    user = 'https://github.com/smontanaro'

    bugs.python.org fields:

    activity = <Date 2017-04-06.18:47:28.725>
    actor = 'vinay.sajip'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-04-06.18:47:28.727>
    closer = 'vinay.sajip'
    components = ['Library (Lib)']
    creation = <Date 2017-03-31.17:18:07.585>
    creator = 'skip.montanaro'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29955
    keywords = []
    message_count = 11.0
    messages = ['290927', '290933', '290942', '290944', '290954', '290958', '290971', '291164', '291227', '291235', '291245']
    nosy_count = 5.0
    nosy_names = ['skip.montanaro', 'rhettinger', 'vinay.sajip', 'pitrou', 'xiang.zhang']
    pr_nums = ['931']
    priority = 'normal'
    resolution = 'not a bug'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29955'
    versions = ['Python 3.7']

    @smontanaro
    Copy link
    Contributor Author

    The logging module hard codes the decimal point for timestamps to be ",". It should use locale.localeconv()["decimal_point"] instead.

    @smontanaro smontanaro added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 31, 2017
    @vsajip
    Copy link
    Member

    vsajip commented Mar 31, 2017

    It's not exactly a decimal point, more a "decimal mark" as per ISO 8601. From the Wikipedia article for the standard at "https://en.wikipedia.org/wiki/ISO_8601#Times -

    "However, a fraction may only be added to the lowest order time element in the representation. A decimal mark, either a comma or a dot (without any preference as stated in resolution 10 of the 22nd General Conference CGPM in 2003, but with a preference for a comma according to ISO 8601:2004) is used as a separator between the time element and its fraction."

    and the citation is

    "ISO 8601:2004(E), ISO, 2004-12-01, 4.2.2.4 ... the decimal fraction shall be divided from the integer part by the decimal sign specified in ISO 31-0, i.e. the comma [,] or full stop [.]. Of these, the comma is the preferred sign."

    Nothing about picking a decimal point based on the current locale.

    @smontanaro
    Copy link
    Contributor Author

    It's Vinay's code, so what he wants should carry the most weight. I did this as much as an exercise in figuring out the whole pull request/bug report process as anything.

    @smontanaro
    Copy link
    Contributor Author

    One example demonstrating that the datetime module at least prefers a decimal point:

    >>> import dateutil.parser
    >>> t = '1993-04-21 08:03:00,123'
    >>> dateutil.parser.parse(t)
    datetime.datetime(1993, 4, 21, 8, 3, 0, 123000)
    >>> dateutil.parser.parse(t).isoformat()
    '1993-04-21T08:03:00.123000'

    Looking at datetime.py, it appears the dot is hard-coded there. Maybe there would be value in the right hand (logging) and the left hand (datetime) doing things the same way?

    @rhettinger
    Copy link
    Contributor

    +1 for changing this to a period. Every time I see the comma, it seems odd to me. It also is harder to parse back into a regular float. In addition, it is hazard when people use commas and delimiters between the fields in a log file.

    @zhangyangyu
    Copy link
    Member

    Although I prefer period but such a change could break backwards compatibility so I don't know it's worth or not. Codes like datetime.strptime('1993-04-21 08:03:00,123', '%Y-%m-%d %H:%M:%S,%f') (usually for such a simple parse goal I like to use the builtin datetime library) would break with this change and IIUC, the datetime format could be customized for logging.

    @vsajip
    Copy link
    Member

    vsajip commented Apr 1, 2017

    I would prefer to keep things as they are - which is conforming to the preferences expressed in ISO 8601, and preserving backwards compatibility. I agree that a period has some advantages, but I went with what the standard said as closely as I could. I'm not sure it has ever been a problem in practice - I don't remember any other issue raising it, and this functionality has been the same since around 2003 IIRC.

    @pitrou
    Copy link
    Member

    pitrou commented Apr 5, 2017

    Since the date format itself isn't localized (fortunately), there is no reason to localize the decimal point either. People wanting a localized logging format can easily override the default configuration. And this proposal would break compatibility with existing log parsing tools.

    We could probably bikeshed the default logging configuration for a long time, but there is value in not changing the defaults from one release to another.

    @vsajip
    Copy link
    Member

    vsajip commented Apr 6, 2017

    I would like to close this issue now, without making changes. Will do in one day, unless someone pipes up.

    @vsajip vsajip added the invalid label Apr 6, 2017
    @smontanaro
    Copy link
    Contributor Author

    Vinay> I would like to close this issue now...

    Go for it.

    As I indicated in a previous comment, the exercise was as much to try and
    come to grips with the process as to actually make the change. There
    certainly appear to be good reasons to leave well enough alone. My primary
    (though minor) concerns at this point are:

    • the datetime module hard-coded it one way (period) while the logging
      package hard-coded it the other way.

    • other logging packages I've used/inherited in other languages
      (admittedly, pretty much Americo-centric) all seem to have used periods.

    This only became an issue for me because I recently started using Flask,
    which sets up the logging environment and provides no straightforward API
    for me to reconfigure its logger. (Peter Otten demonstrated a way to do
    this using functools.partial, which, while doable, certainly doesn't strike
    me as straightforward.) In cases where I'm in complete control, configuring
    my own logging environment makes sense. (In reality, when I'm in complete
    control, I tend to roll my own 20-line Logger class and not use the logging
    module at all, but that's a historical artifact of me discovering
    performance issues several years ago in applications which logged heavily.
    Those issues may well not exist today.)

    Skip

    @vsajip
    Copy link
    Member

    vsajip commented Apr 6, 2017

    and provides no straightforward API for me to reconfigure its logger.

    As far as I know, you just need to do something which configures a Flask logger, then call logging.config.dictConfig() with a suitable configuration which doesn't disable existing loggers and configures the Flask logger how you want. (The dictConfig call should replace any existing configuration with what you've passed to it). See this:

    https://stackoverflow.com/questions/11816236/how-to-config-flask-app-logger-from-a-configure-file

    @vsajip vsajip closed this as completed Apr 6, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants