QuerySet implements `contains` but not `__contains__`

[fidoriel]

This is a similar proposal to #31561, but it is not the same. Currently using x in myQuerySet results in Python using the fallback solution: ​https://docs.python.org/3/reference/expressions.html#membership-test-details because ​https://groups.google.com/g/django-developers/c/NZaMq9BALrs/m/OCNTh6QyCAAJ decided to implement contains in #24141.

I think it is only consistent to have the same behavior implemented in __contains__. I would expect that, it is also a more efficient implementation and unifies Django behavior. Nevertheless, documentation is needed why this inconsistency exists. I was not able to find a reason. Because the mailing list agreed on adding contains, this is discussed behavior. Why was __contains__ not added in the first place? To not have breaking changes? I cannot see what would break.

As said in #31561 a queryset could be a collection to make typing easier. But this is not the intention of this issue.

[Mariusz Felisiak]

Why was __contains__ not added in the first place?

Have you read the discussion that you mention in the ticket? or comments in #24141? The entire discussion is about __contains__ and there was a consensus to add contains() instead.

[Richard Ebeling]

(Collaborator of @fidoriel here)

Background: Type Annotation

One intention of this issue is to improve the usability of type-annotations and static type checking with django code. Due to the missing __contains__ method, QuerySets do not fulfil the typing requirements to be a Container, so passing them to a method that expects a Container will fail static type checking. In consequence, they also do not meet the requirements of Collection and Sequence. This makes type-annotating methods that take, e.g., a list or a queryset, verbose und unnecessarily specific, since you always have to type as "takes a Container[x] OR a QuerySet[x]".

One could argue that this is a problem with Python's abstract base classes and the fact that most ABCs inherit from Container, when most users don't really care if __contains__ is present or not. If it isn't, an element in our_container_like_object test still is valid using the fallback mechanism from above. However, a QuerySet semantically fulfills all the requirements of Container, so I don't think this is a reason to not make it one.

Past Discussion

The discussion in #31561 revolves around Set not being a suitable base class because Sets do not have an order, but QuerySets do. From what I see, it does not give any points against making QuerySet a collections.abc.Collection (by adding __contains__) in general.

In #24141, Carl Meyer said

I think the original PR (to implement QuerySet.__contains__() as .filter(pk=obj.pk).exists() under the hood) is a non-starter

which I understand as criticism towards this specific implementation (__contains__ will be .filter(pk=obj.pk).exists() under the hood, which always hits the database, even if objects are already cached -- this is unexpected in comparison to other helper methods that use the object cache). I don't see any arguments against having __contains__ implemented here.

The discussion in ​https://groups.google.com/g/django-developers/c/NZaMq9BALrs/m/OCNTh6QyCAAJ rejects the idea of implementing .__contains__() identically to the current implementation of .contains(), i.e., not evaluating the queryset. I don't see an argument against having _some_ implementation of __contains__.

The discussion in ​https://github.com/django/django/pull/3906 concludes that x in qs should always evaluate an unevaluated queryset. I don't see an argument to not have __contains()__, just a requirement towards its behavior.

I didn't find arguments against implementing __contains__ in the discussion of ​https://github.com/django/django/pull/13038.

Unless I'm missing something here, there's not really a point against having __contains__ in general. There are the arguments that

• a __contains__ implementation shouldn't always hit the database if objects are already cached (agree)
• len(qs), bool(qs), x in qs should be consistent in whether they evaluate a queryset or not (agree)
• x in qs should evaluate the queryset (I guess debatable depending on perspective, but not our goal in this ticket)

Proposal

In general, it is pythonic to test membership by using in. Currently, the django documentation does not say what happens in this case, you have to be aware of the elementwise-iteration-and-comparison fallback in python and conclude that this iteration implies that the queryset will be evaluated.

If we implemented __contains__ as evalute if not evaluted, then return contains(), we wouldn't have any changes in behavior, but QuerySet would be a Container. Additionally, the documentation could include this, e.g. at ​https://docs.djangoproject.com/en/5.0/ref/models/querysets/#when-querysets-are-evaluated. I think both are desirable.

Thoughts / Did I miss something?

[Anders Kaseorg]

The type annotation use case presented in comment:3 didn’t come up in previous discussion and cannot be satisfied by the addition of .contains() rather than .__contains__(). This argument seems compelling, so I hope I’m not overstepping by reopening for further consideration.

[Natalia Bidart]

Hello Anders, thank for your interest in making Django better but any ticket closed as wontfix should not be reopened without a proper discussion and agreement with the Django community. This process is described ​in these docs:

Always use the forum or mailing list to get a consensus before reopening tickets closed as “wontfix”.