Repository: adafruit/Adafruit_CircuitPython_Logging (language: python)

Issue:
Make true CPython subset?
Now that the library is listed on PyPI, is it worth make this a true subset of the CPython `logging` module?  It would mean moving things around between files, I think a breaking change.  Happy to close if not.


Discussion / hints:
I believe that is the general goal for libraries. If the group agrees, I'm happy to provide manual testing when you get to it. 
@tekktrik just getting to this email now lol! what would have to break? we do use this library a few places, so just want to know what has to get updated post-fix
Here's a list with some comments:

- `level_for()` should probably be private - I think it's only purpose is internal use anyway
- I think `PrintHandler` would become `StreamHandler` with the only `stream` allowable being the standard output
- `FileHandler` would move out of the current `extensions.py` and into `__init__.py`.  At that point this could turn back into a single `adafruit_logging.py` file
- Minor, but I do know `FileHandler` keeps it's stream open rather than using a context manager internally, but figured I'd mention it

I thiiiiiink that's everything.
Oh, in CPython `logging`, `FileHandler` inherits from `StreamHandler`.
@tekktrik sounds good to me, if you're willing to make the updates!
Definitely!

Fix patch:
diff --git a/adafruit_logging/__init__.py b/adafruit_logging.py
similarity index 57%
rename from adafruit_logging/__init__.py
rename to adafruit_logging.py
index f5bcdf4..0217e8b 100644
--- a/adafruit_logging/__init__.py
+++ b/adafruit_logging.py
@@ -51,6 +51,7 @@
 # pylint:disable=invalid-name
 
 import time
+import sys
 
 __version__ = "0.0.0-auto.0"
 __repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_Logger.git"
@@ -63,14 +64,14 @@
     "WARNING",
     "ERROR",
     "CRITICAL",
-    "level_for",
-    "LoggingHandler",
-    "PrintHandler",
+    "_level_for",
+    "Handler",
+    "StreamHandler",
     "logger_cache",
-    "null_logger",
     "getLogger",
     "Logger",
-    "NullLogger",
+    "NullHandler",
+    "FileHandler",
 ]
 
 
@@ -87,7 +88,7 @@
     globals()[__name] = __value
 
 
-def level_for(value: int) -> str:
+def _level_for(value: int) -> str:
     """Convert a numeric level to the most appropriate name.
 
     :param int value: a numeric level
@@ -101,10 +102,16 @@ def level_for(value: int) -> str:
     return LEVELS[0][1]
 
 
-class LoggingHandler:
+#  pylint: disable=too-few-public-methods
+class Handler:
     """Abstract logging message handler."""
 
-    def format(self, log_level: int, message: str) -> str:
+    def __init__(self, level=NOTSET):  # pylint: disable=undefined-variable
+        self.level = level
+        """Level of the handler; this is currently unused, and
+        only the level of the logger is used"""
+
+    def _format(self, log_level: int, message: str) -> str:
         """Generate a timestamped message.
 
         :param int log_level: the logging level
@@ -112,51 +119,63 @@ def format(self, log_level: int, message: str) -> str:
 
         """
         return "{0:<0.3f}: {1} - {2}".format(
-            time.monotonic(), level_for(log_level), message
+            time.monotonic(), _level_for(log_level), message
         )
 
-    def emit(self, log_level: int, message: str):
+    def _emit(self, log_level: int, message: str):
         """Send a message where it should go.
         Placeholder for subclass implementations.
         """
         raise NotImplementedError()
 
 
-class PrintHandler(LoggingHandler):
-    """Send logging messages to the console by using print."""
+#  pylint: disable=too-few-public-methods
+class StreamHandler(Handler):
+    """Send logging messages to a stream, `sys.stderr` (typically
+    the serial console) by default.
+
+    :param stream: The stream to log to, default is `sys.stderr`
+    """
+
+    def __init__(self, stream=None):
+        super().__init__()
+        if stream is None:
+            stream = sys.stderr
+        self.stream = stream
+        """The stream to log to"""
 
-    def emit(self, log_level: int, message: str):
+    def _emit(self, log_level: int, message: str):
         """Send a message to the console.
 
         :param int log_level: the logging level
         :param str message: the message to log
 
         """
-        print(self.format(log_level, message))
+        print(self._format(log_level, message))
 
 
 # The level module-global variables get created when loaded
 # pylint:disable=undefined-variable
 
 logger_cache = {}
-null_logger = None
+
+
+def _addLogger(logger_name: str):
+    """Adds the logger if it doesn't already exist"""
+    if logger_name not in logger_cache:
+        logger_cache[logger_name] = Logger(logger_name)
+
 
 # pylint:disable=global-statement
 def getLogger(logger_name: str) -> "Logger":
-    """Create or retrieve a logger by name.
+    """Create or retrieve a logger by name; only retrieves loggers
+    made using this function; if a Logger with this name does not
+    exist it is created
 
-    :param str logger_name: The name of the `Logger` to create/retrieve. `None`
-                            will cause the `NullLogger` instance to be returned.
+    :param str logger_name: The name of the `Logger` to create/retrieve.
 
     """
-    global null_logger
-    if not logger_name or logger_name == "":
-        if not null_logger:
-            null_logger = NullLogger()
-        return null_logger
-
-    if logger_name not in logger_cache:
-        logger_cache[logger_name] = Logger()
+    _addLogger(logger_name)
     return logger_cache[logger_name]
 
 
@@ -166,10 +185,13 @@ def getLogger(logger_name: str) -> "Logger":
 class Logger:
     """Provide a logging api."""
 
-    def __init__(self):
+    def __init__(self, name: str, level=NOTSET):
         """Create an instance."""
-        self._level = NOTSET
-        self._handler = PrintHandler()
+        self._level = level
+        self.name = name
+        """The name of the logger, this should be unique for proper
+        functionality of `getLogger()`"""
+        self._handler = None
 
     def setLevel(self, log_level: int):
         """Set the logging cutoff level.
@@ -187,86 +209,89 @@ def getEffectiveLevel(self) -> int:
         """
         return self._level
 
-    def addHandler(self, handler: LoggingHandler):
+    def addHandler(self, hdlr: Handler):
         """Sets the handler of this logger to the specified handler.
+
         *NOTE* this is slightly different from the CPython equivalent which adds
         the handler rather than replacing it.
 
         :param LoggingHandler handler: the handler
 
         """
-        self._handler = handler
+        self._handler = hdlr
 
-    def log(self, log_level: int, format_string: str, *args):
+    def hasHandlers(self) -> bool:
+        """Whether any handlers have been set for this logger"""
+        return self._handler is not None
+
+    def log(self, level: int, msg: str, *args):
         """Log a message.
 
-        :param int log_level: the priority level at which to log
-        :param str format_string: the core message string with embedded
+        :param int level: the priority level at which to log
+        :param str msg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        if log_level >= self._level:
-            self._handler.emit(log_level, format_string % args)
+        if level >= self._level:
+            self._handler._emit(level, msg % args)  #  pylint: disable=protected-access
 
-    def debug(self, format_string: str, *args):
+    def debug(self, msg: str, *args):
         """Log a debug message.
 
-        :param str format_string: the core message string with embedded
+        :param str fmsg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        self.log(DEBUG, format_string, *args)
+        self.log(DEBUG, msg, *args)
 
-    def info(self, format_string: str, *args):
+    def info(self, msg: str, *args):
         """Log a info message.
 
-        :param str format_string: the core message string with embedded
+        :param str msg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        self.log(INFO, format_string, *args)
+        self.log(INFO, msg, *args)
 
-    def warning(self, format_string: str, *args):
+    def warning(self, msg: str, *args):
         """Log a warning message.
 
-        :param str format_string: the core message string with embedded
+        :param str msg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        self.log(WARNING, format_string, *args)
+        self.log(WARNING, msg, *args)
 
-    def error(self, format_string: str, *args):
+    def error(self, msg: str, *args):
         """Log a error message.
 
-        :param str format_string: the core message string with embedded
+        :param str msg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        self.log(ERROR, format_string, *args)
+        self.log(ERROR, msg, *args)
 
-    def critical(self, format_string: str, *args):
+    def critical(self, msg: str, *args):
         """Log a critical message.
 
-        :param str format_string: the core message string with embedded
+        :param str msg: the core message string with embedded
                                   formatting directives
         :param args: arguments to ``format_string.format()``; can be empty
 
         """
-        self.log(CRITICAL, format_string, *args)
+        self.log(CRITICAL, msg, *args)
 
 
-class NullLogger:
-    """Provide an empty logger.
-    This can be used in place of a real logger to more efficiently disable
-    logging."""
+class NullHandler(Handler):
+    """Provide an empty log handler.
 
-    def __init__(self):
-        """Dummy implementation."""
+    This can be used in place of a real log handler to more efficiently disable
+    logging."""
 
     def setLevel(self, log_level: int):
         """Dummy implementation."""
@@ -275,23 +300,59 @@ def getEffectiveLevel(self) -> int:
         """Dummy implementation."""
         return NOTSET
 
-    def addHandler(self, handler: LoggingHandler):
+    def addHandler(self, handler: Handler):
         """Dummy implementation."""
 
     def log(self, log_level: int, format_string: str, *args):
         """Dummy implementation."""
 
-    def debug(self, format_string: str, *args):
+    def debug(self, msg: str, *args):
         """Dummy implementation."""
 
-    def info(self, format_string: str, *args):
+    def info(self, msg: str, *args):
         """Dummy implementation."""
 
-    def warning(self, format_string: str, *args):
+    def warning(self, msg: str, *args):
         """Dummy implementation."""
 
-    def error(self, format_string: str, *args):
+    def error(self, msg: str, *args):
         """Dummy implementation."""
 
-    def critical(self, format_string: str, *args):
+    def critical(self, msg: str, *args):
         """Dummy implementation."""
+
+    def _emit(self, log_level: int, message: str):
+        """Dummy implementation"""
+
+
+class FileHandler(StreamHandler):
+    """File handler for working with log files off of the microcontroller (like
+    an SD card)
+
+    :param str filename: The filename of the log file
+    :param str mode: Whether to write ('w') or append ('a'); default is to append
+    """
+
+    def __init__(self, filename: str, mode: str = "a") -> None:
+        # pylint: disable=consider-using-with
+        super().__init__(open(filename, mode=mode))
+
+    def close(self):
+        """Closes the file"""
+        self.stream.close()
+
+    def _format(self, log_level: int, message: str):
+        """Generate a string to log
+
+        :param level: The level of the message
+        :param msg: The message to format
+        """
+        return super()._format(log_level, message) + "\r\n"
+
+    def _emit(self, log_level: int, message: str):
+        """Generate the message and write it to the UART.
+
+        :param level: The level of the message
+        :param msg: The message to log
+        """
+        self.stream.write(self._format(log_level, message))
diff --git a/adafruit_logging/extensions.py b/adafruit_logging/extensions.py
deleted file mode 100644
index 0ea110a..0000000
--- a/adafruit_logging/extensions.py
+++ /dev/null
@@ -1,51 +0,0 @@
-# SPDX-FileCopyrightText: 2021 Alec Delaney for Adafruit Industries
-#
-# SPDX-License-Identifier: MIT
-
-"""
-`extensions`
-====================================================
-
-CircuitPython logging extension for logging to files
-
-* Author(s): Alec Delaney
-"""
-
-from . import LoggingHandler
-
-__version__ = "0.0.0-auto.0"
-__repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_Logging.git"
-
-
-class FileHandler(LoggingHandler):
-    """File handler for working with log files off of the microcontroller (like
-    an SD card)
-
-    :param filepath: The filepath to the log file
-    :param mode: Whether to write ('w') or append ('a'); default is to append
-    """
-
-    def __init__(self, filepath: str, mode: str = "a"):
-        self.logfile = open(  # pylint: disable=consider-using-with
-            filepath, mode, encoding="utf-8"
-        )
-
-    def close(self):
-        """Closes the file"""
-        self.logfile.close()
-
-    def format(self, log_level: int, message: str):
-        """Generate a string to log
-
-        :param level: The level of the message
-        :param msg: The message to format
-        """
-        return super().format(log_level, message) + "\r\n"
-
-    def emit(self, log_level: int, message: str):
-        """Generate the message and write it to the UART.
-
-        :param level: The level of the message
-        :param msg: The message to log
-        """
-        self.logfile.write(self.format(log_level, message))
diff --git a/examples/logging_filehandler.py b/examples/logging_filehandler.py
index 37e5de6..92f9d0d 100644
--- a/examples/logging_filehandler.py
+++ b/examples/logging_filehandler.py
@@ -7,7 +7,7 @@
 import storage
 import adafruit_sdcard
 import adafruit_logging as logging
-from adafruit_logging.extensions import FileHandler
+from adafruit_logging import FileHandler
 
 # Get chip select pin depending on the board, this one is for the Feather M4 Express
 sd_cs = board.D10
@@ -28,3 +28,6 @@
 
 logger.info("Logger initialized!")
 logger.debug("You can even add debug statements to the log!")
+
+# If you're done with the FileHandler, close it
+file_handler.close()
diff --git a/setup.py b/setup.py
index 60ade1a..428e4c2 100644
--- a/setup.py
+++ b/setup.py
@@ -50,5 +50,5 @@
     keywords="adafruit blinka circuitpython micropython logging logger",
     # You can just specify the packages manually here if your project is
     # simple. Or you can use find_packages().
-    packages=["adafruit_logging"],
+    py_modules=["adafruit_logging"],
 )


Test patch:
diff --git a/examples/logging_simpletest.py b/examples/logging_simpletest.py
index 3286bce..3eb258b 100644
--- a/examples/logging_simpletest.py
+++ b/examples/logging_simpletest.py
@@ -11,6 +11,8 @@
 # This should produce an error output
 
 logger = logging.getLogger("test")
+print_logger = logging.StreamHandler()
+logger.addHandler(print_logger)
 
 logger.setLevel(logging.ERROR)
 logger.info("Info message")
@@ -18,15 +20,9 @@
 
 # This should produce no output
 
-null_logger = logging.getLogger(None)
-
-null_logger.setLevel(logging.ERROR)
-null_logger.info("Info message")
-null_logger.error("Error message")
-
-# This should produce no output
-
-null_logger = logging.getLogger("")
+null_logger = logging.getLogger("null")
+null_handler = logging.NullHandler()
+null_logger.addHandler(null_handler)
 
 null_logger.setLevel(logging.ERROR)
 null_logger.info("Info message")