`FieldInstanceTracker` passes `str` instead of `Field` to `DeferredAttributeTracker`
mthuurne opened this issue · 1 comments
Problem
While adding type annotations, mypy flags a typing inconsistency that I think isn't caused by annotations mistakes but rather by the code itself:
$ mypy model_utils/
model_utils/tracker.py:334: error: Argument 1 to "DeferredAttributeTracker" has incompatible type "str"; expected "Field[Any, Any]"
[arg-type]
field_tracker = DeferredAttributeTracker(field)
^~~~~
The relevant code reads as follows:
self.instance._deferred_fields = self.instance.get_deferred_fields()
for field in self.instance._deferred_fields:
field_obj = self.instance.__class__.__dict__.get(field)
if isinstance(field_obj, FileDescriptor):
field_tracker = FileDescriptorTracker(field_obj.field)
setattr(self.instance.__class__, field, field_tracker)
else:
field_tracker = DeferredAttributeTracker(field)
setattr(self.instance.__class__, field, field_tracker)
For FileDescriptorTracker
, a Field
instance is passed to the constructor and this type checks fine. For DeferredAttributeTracker
however, a str
is passed while a Field
instance is expected.
The Django docs confirm that get_deferred_fields()
is supposed to return field names, not the actual fields. The fact that field
is being used as a key for __dict__
supports this.
I can't find documentation for django.db.models.query_utils.DeferredAttribute
, but the implementation uses self.field.attname
at some point, which suggests that field
is a Field
instance rather than a str
. That is consistent with django-stubs
.
The else
clause doesn't seem to have test coverage: all tests still passed after I inserted assert False
there.
So all the clues point to this being a bug. I'm not sure how to fix it though: passing field_obj
to the DeferredAttributeTracker
constructor would satisfy mypy, but without a test I don't know if it would do the right thing at runtime.
Environment
- Django Model Utils version: current
master
branch - Django version: 4.1.7
- Python version: 3.10.6
- Other libraries used, if any: mypy 1.1.1, django-stubs 1.16.0
I think passing field_obj
is not the correct solution after all, as field_obj
is actually a deferred attribute, not the field itself. So we should be passing field_obj.field
to DeferredAttributeTracker
, just like is done for FileDescriptorTracker
.