address review comments, cleanup tests

felixdivo · felixdivo · commit 2dda6238d6b1 · 2019-08-16T02:10:17.000+02:00
diff --git a/can/io/generic.py b/can/io/generic.py
@@ -33,7 +33,7 @@ def __init__(self, file: can.typechecking.AcceptedIOType, mode: str = "rt") -> N
             self.file = cast(Optional[can.typechecking.FileLike], file)
         else:
             # file is some path-like object
-            self.file = open(cast(can.typechecking.PathLike, file), mode)
+            self.file = open(cast(can.typechecking.StringPathLike, file), mode)
 
         # for multiple inheritance
         super().__init__()
diff --git a/can/io/logger.py b/can/io/logger.py
@@ -42,12 +42,13 @@ class Logger(BaseIOHandler, Listener):  # pylint: disable=abstract-method
 
     @staticmethod
     def __new__(
-        cls, filename: typing.Optional[can.typechecking.PathLike], *args, **kwargs
+        cls, filename: typing.Optional[can.typechecking.StringPathLike], *args, **kwargs
     ):
         """
         :param filename: the filename/path of the file to write to,
                          may be a path-like object or None to
                          instantiate a :class:`~can.Printer`
+        :raises ValueError: if the filename's suffix is of an unknown file type
         """
         if filename is None:
             return Printer(*args, **kwargs)
@@ -64,4 +65,4 @@ def __new__(
         try:
             return lookup[suffix](filename, *args, **kwargs)
         except KeyError:
-            raise ValueError(f'unknown file type "{suffix}"')
+            raise ValueError(f'No write support for this unknown log format "{suffix}"')
diff --git a/can/io/player.py b/can/io/player.py
@@ -10,7 +10,8 @@
 from time import time, sleep
 import typing
 
-import can  # pylint: disable=unused-import
+if typing.TYPE_CHECKING:
+    import can
 
 from .generic import BaseIOHandler
 from .asc import ASCReader
@@ -49,21 +50,22 @@ class LogReader(BaseIOHandler):
     def __new__(cls, filename: "can.typechecking.PathLike", *args, **kwargs):
         """
         :param filename: the filename/path of the file to read from
+        :raises ValueError: if the filename's suffix is of an unknown file type
         """
         suffix = pathlib.PurePath(filename).suffix
 
-        if suffix == ".asc":
-            return ASCReader(filename)
-        if suffix == ".blf":
-            return BLFReader(filename)
-        if suffix == ".csv":
-            return CSVReader(filename)
-        if suffix == ".db":
-            return SqliteReader(filename, *args, **kwargs)
-        if suffix == ".log":
-            return CanutilsLogReader(filename)
-
-        raise NotImplementedError(f"No read support for this log format: {filename}")
+        lookup = {
+            ".asc": ASCReader,
+            ".blf": BLFReader,
+            ".csv": CSVReader,
+            ".db": SqliteReader,
+            ".log": CanutilsLogReader,
+        }
+        suffix = pathlib.PurePath(filename).suffix
+        try:
+            return lookup[suffix](filename, *args, **kwargs)
+        except KeyError:
+            raise ValueError(f'No read support for this unknown log format "{suffix}"')
 
 
 class MessageSync:  # pylint: disable=too-few-public-methods
@@ -81,7 +83,8 @@ def __init__(
         """Creates an new **MessageSync** instance.
 
         :param messages: An iterable of :class:`can.Message` instances.
-        :param timestamps: Use the messages' timestamps. If False, uses the *gap* parameter as the time between messages.
+        :param timestamps: Use the messages' timestamps. If False, uses the *gap* parameter
+                           as the time between messages.
         :param gap: Minimum time between sent messages in seconds
         :param skip: Skip periods of inactivity greater than this (in seconds).
         """
diff --git a/can/typechecking.py b/can/typechecking.py
@@ -3,7 +3,8 @@
 
 import typing
 
-import os  # pylint: disable=unused-import
+if typing.TYPE_CHECKING:
+    import os
 
 import mypy_extensions
 
diff --git a/test/listener_test.py b/test/listener_test.py
@@ -111,9 +111,11 @@ def test_filetype_to_instance(extension, klass):
         test_filetype_to_instance(".db", can.SqliteReader)
         test_filetype_to_instance(".log", can.CanutilsLogReader)
 
-        # test file extensions that are not supported
-        with self.assertRaisesRegex(NotImplementedError, ".xyz_42"):
-            test_filetype_to_instance(".xyz_42", can.Printer)
+    def testPlayerTypeResolutionUnsupportedFileTypes(self):
+        for should_fail_with in ["", ".", ".some_unknown_extention_42"]:
+            with self.assertRaises(ValueError):
+                with can.LogReader(should_fail_with):  # make sure we close it anyways
+                    pass
 
     def testLoggerTypeResolution(self):
         def test_filetype_to_instance(extension, klass):
@@ -138,10 +140,10 @@ def test_filetype_to_instance(extension, klass):
         with can.Logger(None) as logger:
             self.assertIsInstance(logger, can.Printer)
 
-        should_fail_with = ["", ".", ".some_unknown_extention_42"]
-        for supposed_fail in should_fail_with:
+    def testLoggerTypeResolutionUnsupportedFileTypes(self):
+        for should_fail_with in ["", ".", ".some_unknown_extention_42"]:
             with self.assertRaises(ValueError):
-                with can.Logger(supposed_fail):  # make sure we close it anyways
+                with can.Logger(should_fail_with):  # make sure we close it anyways
                     pass
 
     def testBufferedListenerReceives(self):
