sympy.Tuple is not an instance of collections.abc.Sequence

[obackhouse]

isinstance(..., Sequence) does not evaluate to True for sympy.Tuple as it does for the builtin tuple. Is this desired behaviour? It seems counterintuitive to me.

Input:

from collections.abc import Container, Sequence
import sympy

x = [1, 2, 3]
print(x, isinstance(x, Container), isinstance(x, Sequence))

x = (1, 2, 3)
print(x, isinstance(x, Container), isinstance(x, Sequence))

x = sympy.Tuple(1, 2, 3)
print(x, isinstance(x, Container), isinstance(x, Sequence))

Output:

[1, 2, 3] True True
(1, 2, 3) True True
(1, 2, 3) True False

[oscarbenjamin]

Maybe this is a bug in collections.abc. You can see the definition of Sequence here:
https://github.com/python/cpython/blob/af5bb1fba49c1f703890e28f69f0928109ecd815/Lib/_collections_abc.py#L999-L1004
Apparently it subclasses Reversible and Collection:

In [1]: from collections.abc import Sequence, Collection, Reversible

In [2]: t = Tuple(1, 2)

In [3]: isinstance(t, Collection)
Out[3]: True

In [4]: isinstance(t, Reversible)
Out[4]: False

Now reversed does work:

In [5]: list(reversed(t))
Out[5]: [2, 1]

So maybe Reversible should be improved to handle this case. Apparently it requires the __reversed__ method even though reversed can function without it:
https://github.com/python/cpython/blob/af5bb1fba49c1f703890e28f69f0928109ecd815/Lib/_collections_abc.py#L332-L345
I tried adding the __reversed__ method:

diff --git a/sympy/core/containers.py b/sympy/core/containers.py
index 34c389b..3430908 100644
--- a/sympy/core/containers.py
+++ b/sympy/core/containers.py
@@ -55,6 +55,9 @@ def __new__(cls, *args, **kwargs):
         obj = Basic.__new__(cls, *args)
         return obj
 
+    def __reversed__(self):
+        return self[::-1]
+
     def __getitem__(self, i):
         if isinstance(i, slice):
             indices = i.indices(len(self))

Now I get:

In [3]: isinstance(t, Collection)
Out[3]: True

In [4]: isinstance(t, Reversible)
Out[4]: True

In [5]: isinstance(t, Sequence)
Out[5]: False

So why is it still false for Sequence? The doc for Sequence says

    Concrete subclasses must override __new__ or __init__,
    __getitem__, and __len__.

Tuple has all of those except __init__ so why doesn't isinstance(t, Sequence) detect that?