From bfaec6595bd93c0213c519ce9e6ecdca343097fc Mon Sep 17 00:00:00 2001
From: Nicholas Junge <n.junge@appliedai.de>
Date: Mon, 18 Mar 2024 15:44:29 +0100
Subject: [PATCH] Make `Transform` an abstract base class, delete iapply method
 ... (#118)

...based on the `invertible` attribute.

The iapply() itself is not abstract since it is strictly speaking not
needed for a transform.
---
 src/nnbench/io/transforms.py | 71 ++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/src/nnbench/io/transforms.py b/src/nnbench/io/transforms.py
index cf976792..a4a3cd8f 100644
--- a/src/nnbench/io/transforms.py
+++ b/src/nnbench/io/transforms.py
@@ -1,5 +1,6 @@
 """Metaclasses for defining transforms acting on benchmark records."""
 
+from abc import ABC, abstractmethod
 from typing import Sequence
 
 from nnbench.types import BenchmarkRecord
@@ -8,19 +9,18 @@
 class Transform:
     """The basic transform which every transform has to inherit from."""
 
-    pass
-
-
-class OneToOneTransform(Transform):
     invertible: bool = True
     """
     Whether this transform is invertible,
     i.e. records can be converted back and forth with no changes or data loss.
     """
+    pass
 
+
+class OneToOneTransform(ABC, Transform):
+    @abstractmethod
     def apply(self, record: BenchmarkRecord) -> BenchmarkRecord:
-        """
-        Apply this transform to a benchmark record.
+        """Apply this transform to a benchmark record.
 
         Parameters
         ----------
@@ -34,8 +34,7 @@ def apply(self, record: BenchmarkRecord) -> BenchmarkRecord:
         """
 
     def iapply(self, record: BenchmarkRecord) -> BenchmarkRecord:
-        """
-        Apply the inverse of this transform.
+        """Apply the inverse of this transform.
 
         In general, applying the inverse on a record not previously transformed
         may yield unexpected results.
@@ -49,25 +48,26 @@ def iapply(self, record: BenchmarkRecord) -> BenchmarkRecord:
         -------
         BenchmarkRecord
             The inversely transformed benchmark record.
+
+        Raises
+        ------
+        RuntimeError
+            If the `Transform.invertible` attribute is set to `False`.
         """
+        if not self.invertible:
+            raise RuntimeError(f"{self.__class__.__name__}() is marked as not invertible")
+        raise NotImplementedError
 
 
 class ManyToOneTransform(Transform):
-    """
-    A many-to-one transform reducing a collection of records to a single record.
+    """A many-to-one transform reducing a collection of records to a single record.
 
     This is useful for computing statistics on a collection of runs.
     """
 
-    invertible: bool = True
-    """
-    Whether this transform is invertible,
-    i.e. records can be converted back and forth with no changes or data loss.
-    """
-
+    @abstractmethod
     def apply(self, record: Sequence[BenchmarkRecord]) -> BenchmarkRecord:
-        """
-        Apply this transform to a benchmark record.
+        """Apply this transform to a benchmark record.
 
         Parameters
         ----------
@@ -82,8 +82,7 @@ def apply(self, record: Sequence[BenchmarkRecord]) -> BenchmarkRecord:
         """
 
     def iapply(self, record: BenchmarkRecord) -> Sequence[BenchmarkRecord]:
-        """
-        Apply the inverse of this transform.
+        """Apply the inverse of this transform.
 
         In general, applying the inverse on a record not previously transformed
         may yield unexpected results.
@@ -97,31 +96,32 @@ def iapply(self, record: BenchmarkRecord) -> Sequence[BenchmarkRecord]:
         -------
         Sequence[BenchmarkRecord]
             The inversely transformed benchmark record sequence.
+
+        Raises
+        ------
+        RuntimeError
+            If the `Transform.invertible` attribute is set to `False`.
         """
-        # TODO: Does this even make sense? Can't hurt to allow it on paper, though.
+        if not self.invertible:
+            raise RuntimeError(f"{self.__class__.__name__}() is marked as not invertible")
+        raise NotImplementedError
 
 
 class ManyToManyTransform(Transform):
-    """
-    A many-to-many transform mapping an input record collection to an output collection.
+    """A many-to-many transform mapping an input record collection to an output collection.
 
     Use this to programmatically wrangle metadata or types in records, or to
     convert parameters into database-ready representations.
     """
 
-    invertible: bool = True
-    """
-    Whether this transform is invertible,
-    i.e. records can be converted back and forth with no changes or data loss.
-    """
     length_invariant: bool = True
     """
     Whether this transform preserves the number of records, i.e. no records are dropped.
     """
 
+    @abstractmethod
     def apply(self, record: Sequence[BenchmarkRecord]) -> Sequence[BenchmarkRecord]:
-        """
-        Apply this transform to a benchmark record.
+        """Apply this transform to a benchmark record.
 
         Parameters
         ----------
@@ -135,8 +135,7 @@ def apply(self, record: Sequence[BenchmarkRecord]) -> Sequence[BenchmarkRecord]:
         """
 
     def iapply(self, record: Sequence[BenchmarkRecord]) -> Sequence[BenchmarkRecord]:
-        """
-        Apply the inverse of this transform.
+        """Apply the inverse of this transform.
 
         In general, applying the inverse on a record not previously transformed
         may yield unexpected results.
@@ -150,4 +149,12 @@ def iapply(self, record: Sequence[BenchmarkRecord]) -> Sequence[BenchmarkRecord]
         -------
         Sequence[BenchmarkRecord]
             The inversely transformed benchmark record sequence.
+
+        Raises
+        ------
+        RuntimeError
+            If the `Transform.invertible` attribute is set to `False`.
         """
+        if not self.invertible:
+            raise RuntimeError(f"{self.__class__.__name__}() is marked as not invertible")
+        raise NotImplementedError