Typing of array_slice as Sequence in connection methods

[markedwards]

I'm implementing graphql_relay in a GraphQL implementation to handle Connection responses by passing django.db.models.QuerySet objects to the array_slice argument of connection_from_array_slice(), much in the same way that Graphene does.

Although I have assured myself that this works, the typing of array_slice as Sequence is a bit disconcerting, since QuerySet does not qualify as a Sequence instance. As far as I can tell, there isn't an appropriate standard type that would solve this, however. The best idea I have would be to define a custom Protocol, but that might pose compatibility challenges for older python installs.

Is there any appetite to resolve this so that a class like QuerySet type-checks without having to cast or ignore it? Barring that, can it at least be stated that graphql_relay will continue to support QuerySet in this way? I assume that is the intention since Graphene relies on it.

[Cito]

The problem is that django.db.models.QuerySet does not inherit from collections.abc.Sequence, although it obviously qualifies as a sequence (so you must not worry). As a solution, you can do a typing.cast(Sequence, query_set) when passing the query set to graphql-relay-py.

I agree a proper solution would use structural subtyping using a Protocol. Ideally, Sequence should already be a Protocol, but unfortunately it isn't and as far as I see there is no other predefined protocol for sliceable objects. So we would need to define a custom one, as you suggested. (Actually I think it's weird that there is no predefined protocol for such a commong thing - did we overlook something obvious?) The other problem is that potocols are only supported since Python 3.8 or would require typing_extensions an additional dependency for Python 3.6 and 3.7 which we still want to support.

I'm leaving this open for discussion and suggestions.

[markedwards]

Unfortunately QuerySet does not qualify as a Sequence, because it lacks __contains__, index, and __reversed__. So, it's risky to provide a QuerySet instance to connection_from_array_slice(), since the code will crash if an attempt is made to call any of those (which is what type hinting exists to avoid). I basically have to read the code to convince myself that it doesn't ever call those methods, and do it again if graphql-relay ever changes.

Unfortunately there is no Indexable type that only implies __getitem__, because if there were you could declare array_slice as Union[Indexable, Sequence], and require array_slice_length to be passed, or raise if it isn't passed and array_slice is not a Sequence.

Defining a Protocol seems like the only proper solution, but I understand why that might not be palatable at this stage. As you suggested, I am currently doing cast(Sequence, query_set), and it's certainly not the worst problem in the world. Just please don't add a call to any of the above methods :-)

[Cito]

Yes, actually you only need __getitem__ and __len__ which are only the "base" methods of Sequence (the other ones can be mixed in using the abstract base class, but QuerySet doesn't do that). The docstring says a sliceable object is required, which hopefully makes a bit clearer what is required.

[markedwards]

Added a PR to demonstrate a solution using Protocol. I don't see any other way, but perhaps I've missed something.