charlesdaniels / bitshuffle

BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

Use logging library #62

Open jyn514 opened 6 years ago

jyn514 commented 6 years ago

We've got sufficient logging that handrolling our output is probably more effort than it's worth. Python has a builtin logging library that will be suitable; it supports debug through critical levels with a special exception level for backtraces.

This will allow interoperability with other libraries and reduce the size of the codebase considerably.

jyn514 commented 6 years ago

Do we want a datestamp for all logs? It will make it more verbose but possibly help with automated runs.

jyn514 commented 6 years ago

Hmm, there doesn't seem to be a builtin way to logically seperate parts of the message like we were doing before.

Currently:

def warn(integer, *argv, **kwargs):
    error = "%s %d: %s" % (LEVELS[kwargs['severity']], integer, ERRORS[integer])
    for arg in argv:
        error += ": " + arg

To do this in the logger, we would have to call our own method every time, we couldn't use the builtin logger.warning method.

>>> import logging
>>> logging.basicConfig(format='%(levelname)s %d: %(message)s')
>>> logging.warn('hi')                                         
--- Logging error ---
Traceback (most recent call last):
...
  File "/usr/local/lib/python3.6/logging/__init__.py", line 391, in format
    return self._fmt % record.__dict__
TypeError: not enough arguments for format string
Call stack:
  File "<stdin>", line 1, in <module>
Message: 'hi'
Arguments: ()
>>> logging.warn('hi', 3)
--- Logging error ---
Traceback (most recent call last):
...
  File "/usr/local/lib/python3.6/logging/__init__.py", line 338, in getMessage
    msg = msg % self.args
TypeError: not all arguments converted during string formatting
Call stack:
  File "<stdin>", line 1, in <module>
Message: 'hi'
Arguments: (3,)
jyn514 commented 6 years ago

Going to rewrite warn so that it works with logger. It does mean we'll have to log like this: logger.warn(logging_format("some message", 0)), but it's better than messing around with instances of logging.Logger and logging.Formatter.

charlesdaniels commented 6 years ago

I seem to recall there was a way to setup a formatter and make it the default. I don’t remember quite how to do this, but I think it would do what we want.

I would not do date stamps, since Bitshuffle should never be long running.

On Mar 4, 2018, at 08:48, Joshua Nelson notifications@github.com wrote:

Going to rewrite warn so that it works with logger. It does mean we'll have to log like this: logger.warn(logging_format("some message", 0)), but it's better than messing around with instances of logging.Logger and logging.Formatter.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

jyn514 commented 6 years ago

right, I was looking at logging.basicConfig but it doesn't accept arbitrary arguments, the strings are predefined in __init__.py

jyn514 commented 6 years ago

got basics working, want to make debug levels consistent in both files before merging