Add 'singledispatch' to the __subclasscheck__ whitelist in the python2.7 port

[adetokunbo]

Hi,

I'm trying to add python2.7 support to cattrs, which is an experimental library for converting Python data (see rtd for more detailed info).

cattrs uses typing.py for runtime conversions, along with singledispatch.

For the main python3 version of typing.py this works as expected. However, there is a discrepancy between the python2 port of typing.py and python3 version that breaks use of singledispatch in python2.

This check,

    def __subclasscheck__(self, cls):
        if self.__origin__ is not None:
            if sys._getframe(1).f_globals['__name__'] not in ['abc', 'functools']:
                raise TypeError("Parameterized generics cannot be used with class "
                                "or instance checks")
      ...

determines if a TypeError should be thrown when issubclass() is called on the parameterized type. If the calling code is from the 'functools' or 'abc' module then it's allowed. In python3, this allows the singledispatch function to dispatch on parametrized generic types.

However, in python2.7, the singledispatch function is not part of the functools module. It's provided in in a separate backport module named 'singledispatch'. Hence, that check currently fails in python2.7 in situations it would pass in python3.x.

A simple fix would be to add 'singledispatch' to the whitelist in the python2 port:

    def __subclasscheck__(self, cls):
        if self.__origin__ is not None:
            if sys._getframe(1).f_globals['__name__'] not in ['abc', 'functools', 'singledispatch']:
                raise TypeError("Parameterized generics cannot be used with class "
                                "or instance checks")
      ...

Please let me know if this change makes sense, I'm quite happy to send a PR to make the change.
If so, I'd like to try and get it done in time for the (3.6.1 release)[#403].

[ilevkivskyi]

There is a problem with isinstance() and issubclass() with subscripted generics: they are not reliable. For example, isinstance([1, 2, 3]. List[str]) could return True. This is why such use case was prohibited. The exclusion for abc and functools is a temporary workaround. Most probably, it will be prohibited also for these modules as well in Python 3.7.

If third party libraries want to use instance and class checks against subscripted generics, then they should develop their own functionality (issubtype?).

Finally, no changes will appear in typing 3.6.1, since this version should match exactly the typing module in Python 3.6.1 (the latter was already released).

[JelleZijlstra]

Would typing_inspect from #377 help here?

[ilevkivskyi]

@JelleZijlstra It will help, but still some work should be done from the third party libraries side. However, if many people will ask for this, and there will be an agreement on semantics, then I think something like issubtype could be added to future versions of typing_inspect.

[adetokunbo]

@ilevkivskyi: Thanks for your explanation. I agree that my proposed changed need not happen and that as @JelleZijlstra suggests typing_inspect looks like the right place for an issubtype to help with this.

How can I vote to add issubtype to #377 ?

After that, the follow-up change that resolves this issue would be for issubtype to be used in functools.singledispatch and its python2 backport. I.e, it seems like part of a change that could remove the current exclusion of abc and functools as a temporary workaround in the __subclasscheck__ method mentioned earlier.

Is that correct ? If so and if issubtype is added to #377, I think I'd be happy to raise a tracking issue and make the necessary changes in a PR to functools and abc if that helps.

[ilevkivskyi]

@adetokunbo

How can I vote to add issubtype to #377 ?

I would postpone this question until at least one version of typing_inspect is released.

After that, the follow-up change that resolves this issue would be for issubtype to be used in functools.singledispatch and its python2 backport. I.e, it seems like part of a change that could remove the current exclusion of abc and functools as a temporary workaround in the __subclasscheck__ method mentioned earlier.

I can't say for sure, but adding anything typing-specific to singledispatch is highly unlikely. But again, if many people will ask for this, then this could be reconsidered.

[Tinche]

Hm. Speaking on behalf of cattrs, we don't actually even need isinstance([1,2,3], List[str]) (or equivalent). In fact, having a for loop inside the isinstance would be an unnecessary speed penalty, since we handle that ourselves anyway, recursively.

This error just happens since we basically do

s = singledispatch(...)
s.register(List, ...)

s(List[int])

so instead of just going directly to singledispatch with List[int], in the future we could check if the given type is a generic type and use the parameterless version.

Hey, it says it's experimental right in the README :)

[adetokunbo]

so instead of just going directly to singledispatch with List[int], in the future we could check if the given type is a generic type and use the parameterless version.

Cool! - that means there should be no need for a change to either python/typing or singledispatch. I'll close this.