Repository: ipfs-shipyard/py-ipfs-http-client

Issue:
Move MatcherSpecInvalidError into `ipfshttpclient/exceptions.py`
https://github.com/ipfs-shipyard/py-ipfs-http-client/blob/09cae7672a39ca3ff991684c1b31cfe7875174c3/ipfshttpclient/filescanner.py#L416-L420

We currently expose all possible exceptions in the `ipfshttpclient/exceptions.py` module. Ideally this one should be moved there to and added into the existing exception hierarchy (inheriting from both some exception there and `TypeError`, I think).

Discussion:

Patch:
diff --git a/ipfshttpclient/exceptions.py b/ipfshttpclient/exceptions.py
index 22d03c2..02b773e 100644
--- a/ipfshttpclient/exceptions.py
+++ b/ipfshttpclient/exceptions.py
@@ -10,13 +10,14 @@ The class hierarchy for exceptions is:
 	 │    ├── EncoderMissingError
 	 │    ├── EncodingError
 	 │    └── DecodingError
-	 └── CommunicationError
-	      ├── ProtocolError
-	      ├── StatusError
-	      ├── ErrorResponse
-	      │    └── PartialErrorResponse
-	      ├── ConnectionError
-	      └── TimeoutError
+	 ├── CommunicationError
+	 │    ├── ProtocolError
+	 │    ├── StatusError
+	 │    ├── ErrorResponse
+	 │    │    └── PartialErrorResponse
+	 │    ├── ConnectionError
+	 │    └── TimeoutError
+	 └── MatcherSpecInvalidError
 
 """
 import typing as ty
@@ -33,25 +34,27 @@ class AddressError(Error, multiaddr.exceptions.Error):  # type: ignore[no-any-un
 	"""Raised when the provided daemon location Multiaddr does not match any
 	of the supported patterns."""
 	__slots__ = ("addr",)
-	#addr: ty.Union[str, bytes]
+
+	addr: ty.Union[str, bytes]
 	
 	def __init__(self, addr: ty.Union[str, bytes]) -> None:
-		self.addr = addr  # type: ty.Union[str, bytes]
+		self.addr = addr
 		Error.__init__(self, "Unsupported Multiaddr pattern: {0!r}".format(addr))
 
 
 class VersionMismatch(Warning):
 	"""Raised when daemon version is not supported by this client version."""
 	__slots__ = ("current", "minimum", "maximum")
-	#current: ty.Sequence[int]
-	#minimum: ty.Sequence[int]
-	#maximum: ty.Sequence[int]
+
+	current: ty.Sequence[int]
+	minimum: ty.Sequence[int]
+	maximum: ty.Sequence[int]
 	
 	def __init__(self, current: ty.Sequence[int], minimum: ty.Sequence[int],
 	             maximum: ty.Sequence[int]) -> None:
-		self.current = current  # type: ty.Sequence[int]
-		self.minimum = minimum  # type: ty.Sequence[int]
-		self.maximum = maximum  # type: ty.Sequence[int]
+		self.current = current
+		self.minimum = minimum
+		self.maximum = maximum
 		
 		msg = "Unsupported daemon version '{}' (not in range: {} ≤ … < {})".format(
 			".".join(map(str, current)), ".".join(map(str, minimum)), ".".join(map(str, maximum))
@@ -65,10 +68,11 @@ class VersionMismatch(Warning):
 class EncoderError(Error):
 	"""Base class for all encoding and decoding related errors."""
 	__slots__ = ("encoder_name",)
-	#encoder_name: str
+
+	encoder_name: str
 	
 	def __init__(self, message: str, encoder_name: str) -> None:
-		self.encoder_name = encoder_name  # type: str
+		self.encoder_name = encoder_name
 		
 		super().__init__(message)
 
@@ -85,10 +89,11 @@ class EncodingError(EncoderError):
 	"""Raised when encoding a Python object into a byte string has failed
 	due to some problem with the input data."""
 	__slots__ = ("original",)
-	#original: Exception
+
+	original: Exception
 	
 	def __init__(self, encoder_name: str, original: Exception) -> None:
-		self.original = original  # type: Exception
+		self.original = original
 		
 		super().__init__("Object encoding error: {}".format(original), encoder_name)
 
@@ -97,27 +102,44 @@ class DecodingError(EncoderError):
 	"""Raised when decoding a byte string to a Python object has failed due to
 	some problem with the input data."""
 	__slots__ = ("original",)
-	#original: Exception
+
+	original: Exception
 	
 	def __init__(self, encoder_name: str, original: Exception) -> None:
-		self.original = original  # type: Exception
+		self.original = original
 		
 		super().__init__("Object decoding error: {}".format(original), encoder_name)
 
 
+##################
+# filescanner.py #
+##################
+
+class MatcherSpecInvalidError(Error, TypeError):
+	"""
+	An attempt was made to build a matcher using matcher_from_spec, but an invalid
+	specification was provided.
+	"""
+
+	def __init__(self, invalid_spec: ty.Any) -> None:
+		super().__init__(
+			f"Don't know how to create a Matcher from spec {invalid_spec!r}"
+		)
+
+
 ###########
 # http.py #
 ###########
 class CommunicationError(Error):
 	"""Base class for all network communication related errors."""
 	__slots__ = ("original",)
-	#original: ty.Optional[Exception]
+
+	original: ty.Optional[Exception]
 	
 	def __init__(self, original: ty.Optional[Exception],
 	             _message: ty.Optional[str] = None) -> None:
-		self.original = original  # type: ty.Optional[Exception]
+		self.original = original
 		
-		msg = ""  # type: str
 		if _message:
 			msg = _message
 		else:
diff --git a/ipfshttpclient/filescanner.py b/ipfshttpclient/filescanner.py
index ddbe8bc..c0a4483 100644
--- a/ipfshttpclient/filescanner.py
+++ b/ipfshttpclient/filescanner.py
@@ -16,6 +16,8 @@ from . import utils
 # To encourage contributions from PyCharm users, we redefine AnyStr.
 #
 # This will get inlined if/when PyCharm no longer flags typing.AnyStr.
+from .exceptions import MatcherSpecInvalidError
+
 AnyStr = ty.TypeVar('AnyStr', bytes, str)
 
 if sys.version_info >= (3, 7):  #PY37+
@@ -413,13 +415,6 @@ match_spec_t = ty.Union[
 ]
 
 
-class MatcherSpecInvalidError(TypeError):
-	def __init__(self, invalid_spec: ty.Any) -> None:
-		super().__init__(
-			f"Don't know how to create a {Matcher.__name__} from spec {invalid_spec!r}"
-		)
-
-
 def _require_spec(spec: ty.Optional[match_spec_t[AnyStr]]) -> match_spec_t[AnyStr]:
 	"""
 	Assist the type checker by narrowing the number of places accepting Optional.


Test patch:
diff --git a/test/unit/test_exceptions.py b/test/unit/test_exceptions.py
new file mode 100644
index 0000000..ec6ac63
--- /dev/null
+++ b/test/unit/test_exceptions.py
@@ -0,0 +1,18 @@
+
+from ipfshttpclient.exceptions import MatcherSpecInvalidError, Error
+from ipfshttpclient.filescanner import Matcher
+
+
+def test_matcher_spec_invalid_error_message():
+	ex = MatcherSpecInvalidError('junk')
+	assert ex.args[0] == f"Don't know how to create a {Matcher.__name__} from spec 'junk'"
+
+
+def test_matcher_spec_invalid_error_multiple_inheritance():
+	ex = MatcherSpecInvalidError('wrong')
+
+	# Base class of all exceptions in this library
+	assert isinstance(ex, Error)
+
+	# Base class of type errors
+	assert isinstance(ex, TypeError)
diff --git a/test/unit/test_filescanner.py b/test/unit/test_filescanner.py
index 716b091..27cb03f 100644
--- a/test/unit/test_filescanner.py
+++ b/test/unit/test_filescanner.py
@@ -6,8 +6,10 @@ import typing as ty
 import pytest
 
 from datetime import datetime
+
 from ipfshttpclient import filescanner
 
+from ipfshttpclient.exceptions import MatcherSpecInvalidError
 from ipfshttpclient.filescanner import FSNodeEntry
 from ipfshttpclient.filescanner import FSNodeType
 
@@ -164,7 +166,7 @@ def test_glob_matching(
 
 @pytest.mark.parametrize('spec', [123, datetime.now()])
 def test_matcher_from_spec_rejects_invalid_spec_type(spec: ty.Any) -> None:
-	with pytest.raises(filescanner.MatcherSpecInvalidError):
+	with pytest.raises(MatcherSpecInvalidError):
 		filescanner.matcher_from_spec(spec)