-
-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FieldInstanceTracker
passes str
instead of Field
to DeferredAttributeTracker
#559
Comments
mthuurne
added a commit
to ProtixIT/django-model-utils
that referenced
this issue
Mar 17, 2023
This makes it pass mypy inspection, but there is no test coverage, so I'm not certain this solution is correct. However, as the code was already broken, it's unlikely to make things worse. jazzband#559
mthuurne
added a commit
to ProtixIT/django-model-utils
that referenced
this issue
Mar 20, 2023
This makes it pass mypy inspection, but there is no test coverage, so I'm not certain this solution is correct. However, as the code was already broken, it's unlikely to make things worse. jazzband#559
mthuurne
added a commit
to ProtixIT/django-model-utils
that referenced
this issue
Mar 21, 2023
This makes it pass mypy inspection, but there is no test coverage, so I'm not certain this solution is correct. However, as the code was already broken, it's unlikely to make things worse. jazzband#559
I think passing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
The relevant code reads as follows:
For
FileDescriptorTracker
, aField
instance is passed to the constructor and this type checks fine. ForDeferredAttributeTracker
however, astr
is passed while aField
instance is expected.The Django docs confirm that
get_deferred_fields()
is supposed to return field names, not the actual fields. The fact thatfield
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 usesself.field.attname
at some point, which suggests thatfield
is aField
instance rather than astr
. That is consistent withdjango-stubs
.The
else
clause doesn't seem to have test coverage: all tests still passed after I insertedassert 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 theDeferredAttributeTracker
constructor would satisfy mypy, but without a test I don't know if it would do the right thing at runtime.Environment
master
branchThe text was updated successfully, but these errors were encountered: