Improve IO class hierarchy + Allow to use SizedRotatingLogger as a context manager (#1147)

* Fix doc
* Add test case for using SizedRotatingLogger as a context manager
* Bump testing tool version
* Improve type hierarchy and interaction
* Update tests
* Improve class hierarchy, documentation, typing and test for rotating loggers
* use tmp_path fixture

Author: felixdivo

can/io/asc.py

@@ -7,7 +7,6 @@
 """
 
 from typing import cast, Any, Generator, IO, List, Optional, Union, Dict
-from can import typechecking
 
 from datetime import datetime
 import time
@@ -16,7 +15,8 @@
 from ..message import Message
 from ..listener import Listener
 from ..util import channel2int
-from .generic import BaseIOHandler
+from .generic import BaseIOHandler, FileIOMessageWriter
+from ..typechecking import AcceptedIOType
 
 
 CAN_MSG_EXT = 0x80000000
@@ -37,7 +37,7 @@ class ASCReader(BaseIOHandler):
 
     def __init__(
         self,
-        file: Union[typechecking.FileLike, typechecking.StringPathLike],
+        file: AcceptedIOType,
         base: str = "hex",
         relative_timestamp: bool = True,
     ) -> None:
@@ -238,7 +238,7 @@ def __iter__(self) -> Generator[Message, None, None]:
         self.stop()
 
 
-class ASCWriter(BaseIOHandler, Listener):
+class ASCWriter(FileIOMessageWriter, Listener):
     """Logs CAN data to an ASCII log file (.asc).
 
     The measurement starts with the timestamp of the first registered message.
@@ -275,7 +275,7 @@ class ASCWriter(BaseIOHandler, Listener):
 
     def __init__(
         self,
-        file: Union[typechecking.FileLike, typechecking.StringPathLike],
+        file: AcceptedIOType,
         channel: int = 1,
     ) -> None:
         """
@@ -286,8 +286,6 @@ def __init__(
                         have a channel set
         """
         super().__init__(file, mode="w")
-        if not self.file:
-            raise ValueError("The given file cannot be None")
 
         self.channel = channel
 
@@ -304,7 +302,6 @@ def __init__(
 
     def stop(self) -> None:
         # This is guaranteed to not be None since we raise ValueError in __init__
-        self.file = cast(IO[Any], self.file)
         if not self.file.closed:
             self.file.write("End TriggerBlock\n")
         super().stop()

can/io/blf.py

@@ -23,7 +23,7 @@
 from ..listener import Listener
 from ..util import len2dlc, dlc2len, channel2int
 from ..typechecking import AcceptedIOType
-from .generic import BaseIOHandler
+from .generic import BaseIOHandler, FileIOMessageWriter
 
 
 class BLFParseError(Exception):
@@ -139,7 +139,7 @@ class BLFReader(BaseIOHandler):
     silently ignored.
     """
 
-    def __init__(self, file):
+    def __init__(self, file: AcceptedIOType) -> None:
         """
         :param file: a path-like object or as file-like object to read from
                      If this is a file-like object, is has to opened in binary
@@ -347,7 +347,7 @@ def _parse_data(self, data):
             pos = next_pos
 
 
-class BLFWriter(BaseIOHandler, Listener):
+class BLFWriter(FileIOMessageWriter, Listener):
     """
     Logs CAN data to a Binary Logging File compatible with Vector's tools.
     """

can/io/canutils.py

@@ -8,8 +8,8 @@
 
 from can.message import Message
 from can.listener import Listener
-from .generic import BaseIOHandler
-
+from .generic import BaseIOHandler, FileIOMessageWriter
+from ..typechecking import AcceptedIOType
 
 log = logging.getLogger("can.io.canutils")
 
@@ -32,7 +32,7 @@ class CanutilsLogReader(BaseIOHandler):
         ``(0.0) vcan0 001#8d00100100820100``
     """
 
-    def __init__(self, file):
+    def __init__(self, file: AcceptedIOType) -> None:
         """
         :param file: a path-like object or as file-like object to read from
                      If this is a file-like object, is has to opened in text
@@ -105,7 +105,7 @@ def __iter__(self):
         self.stop()
 
 
-class CanutilsLogWriter(BaseIOHandler, Listener):
+class CanutilsLogWriter(FileIOMessageWriter, Listener):
     """Logs CAN data to an ASCII log file (.log).
     This class is is compatible with "candump -L".
 
@@ -114,7 +114,7 @@ class CanutilsLogWriter(BaseIOHandler, Listener):
     It the first message does not have a timestamp, it is set to zero.
     """
 
-    def __init__(self, file, channel="vcan0", append=False):
+    def __init__(self, file: AcceptedIOType, channel="vcan0", append=False):
         """
         :param file: a path-like object or as file-like object to write to
                      If this is a file-like object, is has to opened in text

can/io/csv.py

@@ -13,10 +13,55 @@
 
 from can.message import Message
 from can.listener import Listener
-from .generic import BaseIOHandler
+from .generic import BaseIOHandler, FileIOMessageWriter
+from ..typechecking import AcceptedIOType
 
 
-class CSVWriter(BaseIOHandler, Listener):
+class CSVReader(BaseIOHandler):
+    """Iterator over CAN messages from a .csv file that was
+    generated by :class:`~can.CSVWriter` or that uses the same
+    format as described there. Assumes that there is a header
+    and thus skips the first line.
+
+    Any line separator is accepted.
+    """
+
+    def __init__(self, file: AcceptedIOType) -> None:
+        """
+        :param file: a path-like object or as file-like object to read from
+                     If this is a file-like object, is has to opened in text
+                     read mode, not binary read mode.
+        """
+        super().__init__(file, mode="r")
+
+    def __iter__(self):
+        # skip the header line
+        try:
+            next(self.file)
+        except StopIteration:
+            # don't crash on a file with only a header
+            return
+
+        for line in self.file:
+
+            timestamp, arbitration_id, extended, remote, error, dlc, data = line.split(
+                ","
+            )
+
+            yield Message(
+                timestamp=float(timestamp),
+                is_remote_frame=(remote == "1"),
+                is_extended_id=(extended == "1"),
+                is_error_frame=(error == "1"),
+                arbitration_id=int(arbitration_id, base=16),
+                dlc=int(dlc),
+                data=b64decode(data),
+            )
+
+        self.stop()
+
+
+class CSVWriter(FileIOMessageWriter, Listener):
     """Writes a comma separated text file with a line for
     each message. Includes a header line.
 
@@ -37,7 +82,7 @@ class CSVWriter(BaseIOHandler, Listener):
     Each line is terminated with a platform specific line separator.
     """
 
-    def __init__(self, file, append=False):
+    def __init__(self, file: AcceptedIOType, append: bool = False) -> None:
         """
         :param file: a path-like object or a file-like object to write to.
                      If this is a file-like object, is has to open in text
@@ -54,7 +99,7 @@ def __init__(self, file, append=False):
         if not append:
             self.file.write("timestamp,arbitration_id,extended,remote,error,dlc,data\n")
 
-    def on_message_received(self, msg):
+    def on_message_received(self, msg: Message) -> None:
         row = ",".join(
             [
                 repr(msg.timestamp),  # cannot use str() here because that is rounding
@@ -68,47 +113,3 @@ def on_message_received(self, msg):
         )
         self.file.write(row)
         self.file.write("\n")
-
-
-class CSVReader(BaseIOHandler):
-    """Iterator over CAN messages from a .csv file that was
-    generated by :class:`~can.CSVWriter` or that uses the same
-    format as described there. Assumes that there is a header
-    and thus skips the first line.
-
-    Any line separator is accepted.
-    """
-
-    def __init__(self, file):
-        """
-        :param file: a path-like object or as file-like object to read from
-                     If this is a file-like object, is has to opened in text
-                     read mode, not binary read mode.
-        """
-        super().__init__(file, mode="r")
-
-    def __iter__(self):
-        # skip the header line
-        try:
-            next(self.file)
-        except StopIteration:
-            # don't crash on a file with only a header
-            return
-
-        for line in self.file:
-
-            timestamp, arbitration_id, extended, remote, error, dlc, data = line.split(
-                ","
-            )
-
-            yield Message(
-                timestamp=float(timestamp),
-                is_remote_frame=(remote == "1"),
-                is_extended_id=(extended == "1"),
-                is_error_frame=(error == "1"),
-                arbitration_id=int(arbitration_id, base=16),
-                dlc=int(dlc),
-                data=b64decode(data),
-            )
-
-        self.stop()

can/io/generic.py

@@ -30,7 +30,9 @@ class BaseIOHandler(ContextManager, metaclass=ABCMeta):
 
     file: Optional[can.typechecking.FileLike]
 
-    def __init__(self, file: can.typechecking.AcceptedIOType, mode: str = "rt") -> None:
+    def __init__(
+        self, file: Optional[can.typechecking.AcceptedIOType], mode: str = "rt"
+    ) -> None:
         """
         :param file: a path-like object to open a file, a file-like object
                      to be used as a file or `None` to not use a file at all
@@ -77,6 +79,15 @@ class FileIOMessageWriter(MessageWriter, metaclass=ABCMeta):
 
     file: Union[TextIO, BinaryIO]
 
+    def __init__(
+        self, file: Union[can.typechecking.FileLike, TextIO, BinaryIO], mode: str = "rt"
+    ) -> None:
+        # Not possible with the type signature, but be verbose for user friendliness
+        if file is None:
+            raise ValueError("The given file cannot be None")
+
+        super().__init__(file, mode)
+
 
 # pylint: disable=too-few-public-methods
 class MessageReader(BaseIOHandler, Iterable, metaclass=ABCMeta):