Skip to content

Commit

Permalink
Merge pull request #65 from ArcanaFramework/exception-type-cleanup
Browse files Browse the repository at this point in the history
Removed direct references to FileFormatsError in favour of more specific errors
  • Loading branch information
tclose committed Apr 9, 2024
2 parents feeadef + 1afa94d commit 55dc6ea
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 57 deletions.
4 changes: 2 additions & 2 deletions fileformats/core/classifier.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .utils import classproperty
from .exceptions import FileFormatsError
from .exceptions import FormatDefinitionError


class Classifier:
Expand All @@ -17,7 +17,7 @@ def namespace(cls):
namespace"""
module_parts = cls.__module__.split(".")
if module_parts[0] != "fileformats":
raise FileFormatsError(
raise FormatDefinitionError(
f"Cannot create reversible MIME type for {cls} as it is not in the "
"fileformats namespace"
)
Expand Down
6 changes: 3 additions & 3 deletions fileformats/core/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import typing as ty
import logging
from .utils import describe_task, matching_source
from .exceptions import FileFormatsError
from .exceptions import FormatDefinitionError

if ty.TYPE_CHECKING:
from .datatype import DataType
Expand Down Expand Up @@ -127,7 +127,7 @@ def register_converter(
"""
# Ensure "converters" dict is defined in the target class and not in a superclass
if len(source_format.wildcard_classifiers()) > 1:
raise FileFormatsError(
raise FormatDefinitionError(
"Cannot register a conversion to a generic type from a type with more "
f"than one wildcard {source_format} ({list(source_format.wildcard_classifiers())})"
)
Expand All @@ -151,7 +151,7 @@ def register_converter(
describe_task(task),
)
return # actually the same task but just imported twice for some reason
raise FileFormatsError(
raise FormatDefinitionError(
f"Cannot register converter from {source_format} to the generic type "
f"'{tuple(prev_task.wildcard_classifiers())[0]}', {describe_task(task)} "
f"because there is already one registered, {describe_task(prev_task)}"
Expand Down
14 changes: 11 additions & 3 deletions fileformats/core/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
class FileFormatsError(RuntimeError):
class FileFormatsError(Exception):
"Base exception class"


class FormatMismatchError(FileFormatsError):
"File formats don't match"
class FormatDefinitionError(FileFormatsError):
"When the file-formats class hasn't been properly defined"


class FormatMismatchError(TypeError, FileFormatsError):
"Provided paths/values do not match the specified file format"


class UnsatisfiableCopyModeError(FileFormatsError):
"Error copying files"


class FormatConversionError(FileFormatsError):
Expand Down
6 changes: 2 additions & 4 deletions fileformats/core/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
from .utils import (
classproperty,
)
from .exceptions import (
FileFormatsError,
)
from .datatype import DataType
from .exceptions import FormatMismatchError


class Field(DataType):
Expand Down Expand Up @@ -52,7 +50,7 @@ def from_primitive(cls, dtype: type):
datatype = next(iter(f for f in cls.all_fields if f.primitive is dtype))
except StopIteration as e:
field_types_str = ", ".join(t.__name__ for t in cls.all_fields)
raise FileFormatsError(
raise FormatMismatchError(
f"{dtype} doesn't not correspond to a valid fileformats field type "
f"({field_types_str})"
) from e
Expand Down
21 changes: 11 additions & 10 deletions fileformats/core/fileset.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
from .converter import SubtypeVar
from .classifier import Classifier
from .exceptions import (
FileFormatsError,
FormatMismatchError,
UnconstrainedExtensionException,
FormatConversionError,
UnsatisfiableCopyModeError,
FormatDefinitionError,
FileFormatsExtrasError,
FileFormatsExtrasPkgUninstalledError,
FileFormatsExtrasPkgNotCheckedError,
Expand Down Expand Up @@ -106,7 +107,7 @@ def __init__(self, fspaths, metadata=False):

def _validate_fspaths(self):
if not self.fspaths:
raise FileFormatsError(f"No file-system paths provided to {self}")
raise ValueError(f"No file-system paths provided to {self}")
missing = [p for p in self.fspaths if not p or not p.exists()]
if missing:
missing_str = "\n".join(str(p) for p in missing)
Expand Down Expand Up @@ -1343,7 +1344,7 @@ def copy(
)
if constraints:
msg += ", and the following constraints:\n" + "\n".join(constraints)
raise FileFormatsError(msg)
raise UnsatisfiableCopyModeError(msg)
if selected_mode & self.CopyMode.leave:
return self # Don't need to do anything

Expand Down Expand Up @@ -1394,7 +1395,7 @@ def hardlink_dir(src: Path, dest: Path):
else:
os.unlink(new_path)
else:
raise FileFormatsError(
raise FileExistsError(
f"Destination path '{str(new_path)}' exists, set "
"'overwrite' to overwrite it"
)
Expand Down Expand Up @@ -1474,7 +1475,7 @@ def move(
else:
os.unlink(new_path)
else:
raise FileFormatsError(
raise FileExistsError(
f"Destination path '{str(new_path)}' exists, set "
"'overwrite' to overwrite it"
)
Expand Down Expand Up @@ -1523,7 +1524,7 @@ def _fspaths_to_copy(
n for n, c in Counter(p.name for p in self.fspaths).items() if c > 1
]
if duplicate_names:
raise FileFormatsError(
raise UnsatisfiableCopyModeError(
f"Cannot copy/move {self} with collation mode "
f'"{collation}", as there are duplicate filenames, {duplicate_names}, '
f"in file paths: " + "\n".join(str(p) for p in self.fspaths)
Expand All @@ -1535,7 +1536,7 @@ def _fspaths_to_copy(
exts = [d[-1] for d in decomposed_fspaths]
duplicate_exts = [n for n, c in Counter(exts).items() if c > 1]
if duplicate_exts:
raise FileFormatsError(
raise UnsatisfiableCopyModeError(
f"Cannot copy/move {self} with collation mode "
f'"{collation}", as there are duplicate extensions, {duplicate_exts}, '
f"in file paths: " + "\n".join(str(p) for p in self.fspaths)
Expand All @@ -1545,7 +1546,7 @@ def _fspaths_to_copy(
else:
fspaths_to_copy = self.fspaths
if not fspaths_to_copy:
raise FileFormatsError(
raise UnsatisfiableCopyModeError(
f"Cannot copy {self} because none of the fspaths in the file-set are "
"required. Set trim=False to copy all file-paths"
)
Expand Down Expand Up @@ -1645,8 +1646,8 @@ def namespace(cls):
continue
try:
return base.namespace
except FileFormatsError:
except FormatDefinitionError:
pass
raise FileFormatsError(
raise FormatDefinitionError(
f"None of of the bases classes of {cls} ({cls.__mro__}) have a valid namespace"
)
6 changes: 3 additions & 3 deletions fileformats/core/identification.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import typing as ty
import re
from fileformats.core.exceptions import (
FileFormatsError,
FormatDefinitionError,
FormatRecognitionError,
)
import fileformats.core
Expand Down Expand Up @@ -223,7 +223,7 @@ def unwrap(candidate):
ignore_re = re.compile(ignore)
remaining = [p for p in remaining if not ignore_re.match(p.name)]
if remaining:
raise FileFormatsError(
raise FormatRecognitionError(
"the following file-system paths were not recognised by any of the "
f"candidate formats ({candidates_str}):\n"
+ "\n".join(str(p) for p in remaining)
Expand All @@ -233,7 +233,7 @@ def unwrap(candidate):

def to_mime_format_name(format_name: str):
if "___" in format_name:
raise FileFormatsError(
raise FormatDefinitionError(
f"Cannot convert name of format class {format_name} to mime string as it "
"contains triple underscore"
)
Expand Down
35 changes: 20 additions & 15 deletions fileformats/core/mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
from .utils import classproperty, describe_task, matching_source
from .identification import to_mime_format_name
from .converter import SubtypeVar
from .exceptions import FileFormatsError, FormatMismatchError, FormatRecognitionError
from .exceptions import (
FormatMismatchError,
FormatRecognitionError,
FormatDefinitionError,
)


logger = logging.getLogger("fileformats")
Expand Down Expand Up @@ -95,7 +99,7 @@ def version(self) -> ty.Union[str, ty.Tuple[str]]:
)
version = tuple(b.decode("utf-8") for b in match.groups())
if not version:
raise FileFormatsError(
raise FormatDefinitionError(
f"No version patterns found in magic pattern of {type(self).__name__} "
f"class, {self.magic_pattern}"
)
Expand Down Expand Up @@ -278,7 +282,7 @@ def my_func(file: MyFormatWithClassifiers[Integer]):

def _validate_class(self):
if self.wildcard_classifiers():
raise FileFormatsError(
raise FormatDefinitionError(
f"Can instantiate {type(self)} class as it has wildcard classifiers "
"and therefore should only be used for converter specifications"
)
Expand Down Expand Up @@ -317,7 +321,7 @@ def __class_getitem__(cls, classifiers):
if not any(issubclass(q, t) for t in cls.allowed_classifiers)
]
if not_allowed:
raise FileFormatsError(
raise FormatDefinitionError(
f"Invalid content types provided to {cls} (must be subclasses of "
f"{cls.allowed_classifiers}): {not_allowed}"
)
Expand All @@ -339,7 +343,7 @@ def __class_getitem__(cls, classifiers):
repetitions[exc_classifier].append(classifier)
repeated = [t for t in repetitions.items() if len(t[1]) > 1]
if repeated:
raise FileFormatsError(
raise FormatDefinitionError(
"Cannot have more than one occurrence of a classifier "
f"or subclasses for {cls} class when "
f"{cls.__name__}.ordered_classifiers is false:\n"
Expand All @@ -351,8 +355,9 @@ def __class_getitem__(cls, classifiers):
classifiers = frozenset(classifiers)
else:
if len(classifiers) > 1:
raise FileFormatsError(
f"Multiple classifiers not permitted for {cls} types, provided: ({classifiers})"
raise FormatDefinitionError(
f"Multiple classifiers not permitted for {cls} types, provided: "
f"({classifiers})"
)
# Make sure that the "classified" dictionary is present in this class not super
# classes
Expand All @@ -363,26 +368,26 @@ def __class_getitem__(cls, classifiers):
classified = cls._classified_subtypes[classifiers]
except KeyError:
if not hasattr(cls, "classifiers_attr_name"):
raise FileFormatsError(
raise FormatDefinitionError(
f"{cls} needs to define the 'classifiers_attr_name' class attribute "
"with the name of the (different) class attribute to hold the "
"classified types"
)
if cls.classifiers_attr_name is None:
raise FileFormatsError(
raise FormatDefinitionError(
f"Inherited classifiers have been disabled in {cls} (by setting "
f'"classifiers_attr_name)" to None)'
)
try:
classifiers_attr = getattr(cls, cls.classifiers_attr_name)
except AttributeError:
raise FileFormatsError(
raise FormatDefinitionError(
f"Default value for classifiers attribute "
f"'{cls.classifiers_attr_name}' needs to be set in {cls}"
)
else:
if classifiers_attr:
raise FileFormatsError(
raise FormatDefinitionError(
f"Default value for classifiers attribute "
f"'{cls.classifiers_attr_name}' needs to be set in {cls}"
)
Expand Down Expand Up @@ -571,18 +576,18 @@ def register_converter(
if cls.wildcard_classifiers():
if issubclass(source_format, SubtypeVar):
if len(cls.wildcard_classifiers()) > 1:
raise FileFormatsError(
raise FormatDefinitionError(
"Can only have one wildcard qualifier when registering a converter "
f"to {cls} from a generic type, found {cls.wildcard_classifiers()}"
)
elif not source_format.is_classified:
raise FileFormatsError(
raise FormatDefinitionError(
"Can only use wildcard classifiers when registering a converter "
f"from a generic type or similarly classified type, not {source_format}"
)
else:
if cls.wildcard_classifiers() != source_format.wildcard_classifiers():
raise FileFormatsError(
raise FormatDefinitionError(
f"Mismatching wildcards between source format, {source_format} "
f"({list(source_format.wildcard_classifiers())}), and target "
f"{cls} ({cls.wildcard_classifiers()})"
Expand Down Expand Up @@ -612,7 +617,7 @@ def register_converter(
describe_task(task),
)
return # actually the same task but just imported twice for some reason
raise FileFormatsError(
raise FormatDefinitionError(
f"Cannot register converter from {prev.unclassified} "
f"to {cls.unclassified} with non-wildcard classifiers "
f"{list(prev.non_wildcard_classifiers())}, {describe_task(task)}, "
Expand Down
24 changes: 16 additions & 8 deletions fileformats/core/tests/test_classifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from fileformats.generic import DirectoryContaining
from fileformats.field import Array, Integer, Decimal, Text, Boolean
from fileformats.core.exceptions import (
FileFormatsError,
FormatDefinitionError,
FormatConversionError,
FormatRecognitionError,
FormatMismatchError,
Expand Down Expand Up @@ -149,13 +149,16 @@ def test_file_classifiers2():


def test_file_classifiers3():
with pytest.raises(FileFormatsError, match="Invalid content types provided to"):
with pytest.raises(
FormatDefinitionError, match="Invalid content types provided to"
):
H[D]


def test_file_classifiers4():
with pytest.raises(
FileFormatsError, match="Cannot have more than one occurrence of a classifier "
FormatDefinitionError,
match="Cannot have more than one occurrence of a classifier ",
):
H[A, B, A]

Expand All @@ -166,15 +169,15 @@ def test_file_classifiers5():

def test_file_classifiers6():
with pytest.raises(
FileFormatsError,
FormatDefinitionError,
match="Default value for classifiers attribute 'new_classifiers_attr' needs to be set",
):
Q[A]


def test_file_classifiers7():
with pytest.raises(
FileFormatsError, match="Multiple classifiers not permitted for "
FormatDefinitionError, match="Multiple classifiers not permitted for "
):
M[A, B]

Expand Down Expand Up @@ -404,12 +407,16 @@ def test_classifier_categories2():


def test_classifier_categories3():
with pytest.raises(FileFormatsError, match="Cannot have more than one occurrence"):
with pytest.raises(
FormatDefinitionError, match="Cannot have more than one occurrence"
):
Classified[U, V]


def test_classifier_categories4():
with pytest.raises(FileFormatsError, match="Cannot have more than one occurrence"):
with pytest.raises(
FormatDefinitionError, match="Cannot have more than one occurrence"
):
Classified[U, W]


Expand All @@ -419,6 +426,7 @@ def test_classifier_categories5():

def test_classifier_categories6():
with pytest.raises(
FileFormatsError, match="Cannot have more than one occurrence of a classifier "
FormatDefinitionError,
match="Cannot have more than one occurrence of a classifier ",
):
Classified[C, E]
Loading

0 comments on commit 55dc6ea

Please sign in to comment.