Skip to content

Support stricter QuerySet row types for values/values_list via plugin magic #45

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

Closed
6 of 15 tasks
syastrov opened this issue Mar 12, 2019 · 2 comments
Closed
6 of 15 tasks

Comments

@syastrov
Copy link
Contributor

syastrov commented Mar 12, 2019

I'm gathering a list of things necessary to implement good support for adding specific types for values / values_list based on the current QuerySet + arguments to those methods + model information.

Basic support:

  • Lookup fields passed in on QuerySet's model (probably can reuse a lot of the information already used to support field access on model instances).
  • Support 'id' / 'pk' lookups
  • Support passing no arguments to values / values_list. It returns the correct type (Tuple/NamedTuple/Dict), but without field types specified.
  • Add a flag to mypy_django.ini (called e.g. use_strict_queryset_row_types, default false) to turn on the feature. Document it in the README. I decided not to do this, since the fallbacks (e.g. to Any) should not cause problems.

Related fields support:

  • Lookup fields on models related to QuerySet (when using __), recursively.
  • For a related field called related_field, support both related_field and related_field_id as arguments (they should return the related model's PK type).
  • Support models using inheritance.

Other nice-to-have + difficult/dynamic cases:

  • When no arguments are passed to values / values_list, it should return all fields (extra + annotated + model fields?).
  • Support QuerySet's that had previously been annotate-ed. We could keep track of extra annotated fields on the type metadata. It will sometimes be difficult to determine the correct type for some of those. We could try to rely on type-annotating the output_field attribute or property on the appropriate subclass of Expression in the stubs to get the most specific types. Maybe in some cases the plugin will have to do some magic to figure it out if it's possible (e.g. where F expressions are used).
  • Support annotating without an alias specified (e.g. Book.objects.annotate(Max('id')).values_list('id__max', flat=True)). It seems like it uses the lower-cased name attribute of the Aggregate appended to the field expression in the case there is only one expression.
  • Raise an error when annotating with an alias that conflicts with a model field name.
  • values supports adding additional annotations using keyword arguments. Not difficult once annotations are supported.
  • Support expressions passed to values_list (e.g. Entry.objects.values_list('id', Lower('headline')))). Should be doable once annotations are supported.
  • Support lookups in values/values_list. e.g. the docs gives an example with a custom lookup Blog.objects.values('name__lower'). Probably it's only feasible for the built-in lookups. If we have lookup support, then supporting type-checking .filter and Q objects is not far-fetched.
  • What happens when you use select_related / prefetch_related?

Related, but outside of scope for this task:

  • Adding extra fields to Model instances with the correct types if you call annotate on the QuerySet.

Is there anything missing?

@Naddiseo
Copy link
Contributor

An example of the first one that I ran into the other day:

class MyModel(models.Model):
    fk = models.ForeignKey('OtherModel', null=True, default=None)

reveal_type(MyModel().fk) # Optional[OtherModel] # Okay, that's fair

mymodel = MyModel.objects.filter(fk=1).get()
reveal_type(mymodel.fk) # This shouldn't be Optional at this point since the filter will have refined the type

This isn't even restricted to related fields, but any nullable field; the queryset filter can refine the type of the output models.

@syastrov
Copy link
Contributor Author

That sounds like a great idea. I wonder if there any edge cases.

@TonyRippy TonyRippy mentioned this issue Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants