Restore original comments and docstrings

Author: craigds

fieldsignals/signals.py

@@ -41,14 +41,24 @@ def connect(  # type: ignore[override]
         """
 
         if not apps.models_ready:
+            # We require access to Model._meta.get_fields(), which isn't available yet.
+            # (This error would be raised below anyway, but we want a more meaningful message)
             raise AppRegistryNotReady(
                 "django-fieldsignals signals must be connected after the app cache is ready. "
                 "Connect the signal in your AppConfig.ready() handler."
             )
 
+        # Validate arguments
+
         if kwargs.get("weak", False):
+            # TODO: weak refs? I'm hella confused.
+            # We can't go passing our proxy receivers around as weak refs, since they're
+            # defined as closures and hence don't exist by the time they're called.
+            # However, we can probably make _make_proxy_receiver() create weakrefs to
+            # the original receiver if required. Patches welcome
             raise NotImplementedError("This signal doesn't yet handle weak refs")
 
+        # Check it's a class, don't check if it's a model class (useful for tests)
         if not isinstance(sender, type):
             raise ValueError("sender should be a model class")
 
@@ -74,6 +84,7 @@ def is_reverse_rel(f: Any) -> bool:
 
         super().connect(proxy_receiver, sender=sender, weak=False, dispatch_uid=dispatch_uid)
 
+        ### post_init : initialize the list of fields for each instance
         def post_init_closure(sender: type, instance: models.Model, **kwargs: Any) -> None:
             self.get_and_update_changed_fields(receiver, instance, resolved_fields)
 
@@ -89,7 +100,10 @@ def connect_source_signals(self, sender: type) -> None:
         """
         Connects the source signals required to trigger updates for this
         ChangedSignal.
+
+        (post_init has already been connected during __init__)
         """
+        # override in subclasses
         pass
 
     def _make_proxy_receiver(
@@ -100,7 +114,8 @@ def _make_proxy_receiver(
     ) -> Callable[..., Any]:
         """
         Takes a receiver function and creates a closure around it that knows what fields
-        to watch.
+        to watch. The original receiver is called for an instance iff the value of
+        at least one of the fields has changed since the last time it was called.
         """
 
         def pr(instance: Any, *args: Any, **kwargs: Any) -> None:
@@ -115,9 +130,9 @@ def pr(instance: Any, *args: Any, **kwargs: Any) -> None:
 
         pr._original_receiver = receiver  # type: ignore[attr-defined]
         pr._fields = fields  # type: ignore[attr-defined]
+
         pr.__doc__ = receiver.__doc__
         pr.__name__ = receiver.__name__
-
         return pr
 
     def get_and_update_changed_fields(
@@ -128,8 +143,18 @@ def get_and_update_changed_fields(
     ) -> dict[str, tuple[Any, Any]]:
         """
         Takes a receiver and a model instance, and a list of field instances.
-        Returns a dict of changed fields: {"fieldname": ("old value", "new value")}
+        Gets the old and new values for each of the given fields, and stores their
+        new values for next time.
+
+        Returns a dict like this:
+            {
+                "fieldname1" : ("old value", "new value"),
+            }
         """
+        # instance._fieldsignals_originals looks like this:
+        #   {
+        #       (id(<signal instance>), id(<receiver>)) : {"field_name": "old value",},
+        #   }
         key = (id(self), id(receiver))
         if not hasattr(instance, "_fieldsignals_originals"):
             instance._fieldsignals_originals = {}
@@ -143,13 +168,18 @@ def get_and_update_changed_fields(
         for field in fields:
             if field.attname in deferred_fields:
                 continue
+            # using value_from_object instead of getattr() means we don't traverse foreignkeys
             new_value = field.to_python(field.value_from_object(instance))
             old_value = originals.get(field.name, None)
             if old_value != new_value:
                 if not isinstance(new_value, IMMUTABLE_TYPES_WHITELIST):
+                    # For mutable types, make a copy of the value before storing it.
+                    # Otherwise, the 'originals' dict may well get modified elsewhere, and
+                    # that's going to make change detection impossible
                     new_value = deepcopy(new_value)
 
                 changed_fields[field.name] = (old_value, new_value)
+                # now update, for next time
                 originals[field.name] = new_value
         return changed_fields
 
@@ -187,5 +217,9 @@ def connect_source_signals(self, sender: type) -> None:
         )
 
 
+### API:
+
 pre_save_changed = PreSaveChangedSignal()
 post_save_changed = PostSaveChangedSignal()
+
+# TODO other signals?