Handle internal review

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>

model_signing/signing/empty_signing.py

@@ -45,9 +45,11 @@ def from_manifest(cls, manifest: manifest.Manifest) -> Self:
         del manifest  # unused
         return cls()
 
-    def __eq__(self, other: "EmptySigningPayload") -> bool:
-        """Checks that `other` is also an `EmptySigningPayload`."""
-        return isinstance(other, EmptySigningPayload)
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, type(self)):
+            return NotImplemented
+
+        return True  # all instances are equal
 
 
 class EmptySignature(signing.Signature):
@@ -61,7 +63,7 @@ class EmptySignature(signing.Signature):
     """
 
     @override
-    def write_signature(self, path: pathlib.Path) -> None:
+    def write(self, path: pathlib.Path) -> None:
         """Writes the signature to disk, to the given path.
 
         Since the signature is empty this function actually does nothing, it's
@@ -74,7 +76,7 @@ def write_signature(self, path: pathlib.Path) -> None:
 
     @classmethod
     @override
-    def read_signature(cls, path: pathlib.Path) -> Self:
+    def read(cls, path: pathlib.Path) -> Self:
         """Reads the signature from disk.
 
         Since the signature is empty, this does nothing besides just returning
@@ -89,9 +91,11 @@ def read_signature(cls, path: pathlib.Path) -> Self:
         del path  # unused
         return cls()
 
-    def __eq__(self, other: "EmptySignature") -> bool:
-        """Checks that `other` is also an `EmptySignature`."""
-        return isinstance(other, EmptySignature)
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, type(self)):
+            return NotImplemented
+
+        return True  # all instances are equal
 
 
 class EmptySigner(signing.Signer):

model_signing/signing/empty_signing_test.py

@@ -53,9 +53,9 @@ class TestEmptySignature:
 
     def test_write_and_read(self):
         signature = empty_signing.EmptySignature()
-        signature.write_signature(test_support.UNUSED_PATH)
+        signature.write(test_support.UNUSED_PATH)
 
-        new_signature = empty_signing.EmptySignature.read_signature(
+        new_signature = empty_signing.EmptySignature.read(
             test_support.UNUSED_PATH
         )
 
@@ -77,12 +77,12 @@ class _FakeSignature(signing.Signature):
     """A test only signature that does nothing."""
 
     @override
-    def write_signature(self, path: pathlib.Path) -> None:
+    def write(self, path: pathlib.Path) -> None:
         del path  # unused, do nothing
 
     @classmethod
     @override
-    def read_signature(cls, path: pathlib.Path) -> Self:
+    def read(cls, path: pathlib.Path) -> Self:
         del path  # unused, do nothing
         return cls()
 

model_signing/signing/signing.py

@@ -69,7 +69,7 @@ class Signature(metaclass=abc.ABCMeta):
     """Generic signature support."""
 
     @abc.abstractmethod
-    def write_signature(self, path: pathlib.Path) -> None:
+    def write(self, path: pathlib.Path) -> None:
         """Writes the signature to disk, to the given path.
 
         Args:
@@ -79,7 +79,7 @@ def write_signature(self, path: pathlib.Path) -> None:
 
     @classmethod
     @abc.abstractmethod
-    def read_signature(cls, path: pathlib.Path) -> Self:
+    def read(cls, path: pathlib.Path) -> Self:
         """Reads the signature from disk.
 
         Does not perform any signature verification, except what is needed to
@@ -131,9 +131,7 @@ class Verifier(metaclass=abc.ABCMeta):
 
     Every subclass of `Verifier` is paired with a subclass of `Signer`. This is
     to ensure that they support the same signing payload and signature formats
-    as well as have similar key materialsEvery subclass of `Verifier` is paired
-    with a subclass of `Signer`. This is to ensure that they support the same
-    signing payload and signature formats as well as have similar key materials.
+    as well as have similar key materials.
 
     If the signature is valid, the payload is expanded to a `Manifest` instance
     which can then be used to check the model integrity.
@@ -150,7 +148,7 @@ def verify(self, signature: Signature) -> manifest.Manifest:
             A `manifest.Manifest` instance that represents the model.
 
         Raises:
-            ValueError: If the signature cannot be verified.
+            ValueError: If the signature verification fails.
             TypeError: If the signature is not one of the `Signature` subclasses
               accepted by the verifier.
         """