Problem

I was cleaning up the type annotations in DescriptorWrapper.__get__():

    def __get__(self, instance: models.Model | None, owner: type[models.Model]) -> DescriptorWrapper[T] | T:
        if instance is None:
            return self
        was_deferred = self.field_name in instance.get_deferred_fields()
        try:
            value = self.descriptor.__get__(instance, owner)
        except AttributeError:
            value = self.descriptor
        if was_deferred:
            tracker_instance = getattr(instance, self.tracker_attname)
            tracker_instance.saved_data[self.field_name] = lightweight_deepcopy(value)
        return value

After being puzzled by the code for some time, I realized that the value = self.descriptor statement was inherently incompatible with the return type of the method. Also storing the descriptor rather than a value in tracker_instance.saved_data seems unlikely to be correct.

Solution

I tried to find out when this exception handler runs, but it turned out it is never executed during unit testing.

Digging deeper, I found that AbstractModelTrackerTests was supposed to cover this code, but that test suite was marked as skip because it had known test failures. Re-enabling it indeed led to 4 failing test cases, all from ModelTrackerTests. The reason for that was simple though: the test model TrackedAbstract used a FieldTracker rather than a ModelTracker. After fixing that, the test suite passed.

There used to be a bug in Django that caused these tests to fail, according to the discussion of #370. That bug was fixed in Django 4.0.

This exception handler was introduced in #367, but subsequent discussion in #370, #381 and #382 suggests that it wasn't the proper solution. As we now have a working test and that test doesn't cause AttributeError to be raised, I think it is safe to remove the try/except.

Commandments

• Write PEP8 compliant code.
• Cover it with tests.
• Update CHANGES.rst file to describe the changes, and quote according issue with GH-<issue_number>.
• Pay attention to backward compatibility, or if it breaks it, explain why.
• Update documentation (if relevant).

This change should be invisible to end users, as it removes dead code. So I think the ChangeLog and docs steps can be skipped, but if you disagree, let me know and I'll update the PR.