[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:

I am closing this for now. We can re-open it later if problems will appear in 
3.7.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:


New changeset 5d8bb5d07be2a9205e7059090f0ac5360d36b217 by Ivan Levkivskyi (Miss 
Islington (bot)) in branch '3.7':
bpo-32999: Revert GH-6002 (fc7df0e6) (GH-6189) (GH-6190)
https://github.com/python/cpython/commit/5d8bb5d07be2a9205e7059090f0ac5360d36b217


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread miss-islington

Change by miss-islington :


--
pull_requests: +5936

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread INADA Naoki

INADA Naoki  added the comment:


New changeset f757b72b2524ce3451d2269f0b8a9f0593a7b27f by INADA Naoki in branch 
'master':
bpo-32999: Revert GH-6002 (fc7df0e6) (GH-6189)
https://github.com/python/cpython/commit/f757b72b2524ce3451d2269f0b8a9f0593a7b27f


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread INADA Naoki

Change by INADA Naoki :


--
pull_requests: +5934

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread miss-islington

miss-islington  added the comment:


New changeset 346964ba0586e402610ea886e70bee1294874781 by Miss Islington (bot) 
in branch '3.7':
bpo-33018: Improve issubclass() error checking and message. (GH-5944)
https://github.com/python/cpython/commit/346964ba0586e402610ea886e70bee1294874781


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread miss-islington

Change by miss-islington :


--
keywords: +patch
pull_requests: +5932
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-22 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:


New changeset 40472dd42de4f7265d456458cd13ad6894d736db by Ivan Levkivskyi (jab) 
in branch 'master':
bpo-33018: Improve issubclass() error checking and message. (GH-5944)
https://github.com/python/cpython/commit/40472dd42de4f7265d456458cd13ad6894d736db


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-21 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:

> Would you merge this into master?

OK, I played with this a bit and it looks good. There is however a merge 
conflict now, and a NEWS item is missing. I will leave a comment in the PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-21 Thread Guido van Rossum

Guido van Rossum  added the comment:

OTOH if we don't do this now, it's not going to be any easier to make this
change in 3.8. Maybe now's the time to experiment with it, and we can drop
it in rc1 if it causes problems. @Ivan, your thoughts? Would you merge this
into master?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-20 Thread Guido van Rossum

Guido van Rossum  added the comment:

Hmm... That is actually a not entirely imaginary risk, right? Can you come
up with a test program that would fail that way?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-20 Thread INADA Naoki

INADA Naoki  added the comment:

If there are some code which depend on ABC.__subclasscheck__() allow class-like 
object, we'll have to revert it in 3.7b4 or rc.
I think it's the only risk.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-20 Thread Guido van Rossum

Guido van Rossum  added the comment:

While we're in feature freeze mode, we're not in release candidate mode
yet. I presume the code that would be patched in 3.7 is no different from
master? What's the concern? It doesn't seem anyone thinks it's risky?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-20 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:

I am still -1 on changing this in Python 3.7, unless Guido wants this in 3.7, 
if yes, then we can go ahead. Otherwise, I think we can consider just merging 
this into master, in this case I would switch to the PR to discuss the details.

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-12 Thread Joshua Bronson

Joshua Bronson  added the comment:

I'll share the use case that prompted me to submit this PR in the first place.

I am the author of bidict (https://pypi.python.org/pypi/bidict), which provides 
a bidirectional dict class. A bidict is just like a dict, except it 
automatically maintains its inverse bidict, which is accessible via its .inv 
attribute. To prevent a bidict and its inverse from creating a strong reference 
cycle, a weak ref is used to store the reference one direction.

bidicts implement my BidirectionalMapping ABC, which extends 
collections.abc.Mapping to include the .inv abstractproperty. 
BidirectionalMapping overrides __subclasshook__ so that outside implementations 
that don't subclass it explicitly may still be considered subclasses.

Recently, I tried something like `issublass('foo', BidirectionalMapping)`, and 
got the "cannot create weak reference to 'str' object" error. Because this 
error message differs from the (much more helpful) "arg 1 must be a class" 
error message that you get when you do e.g. `issubclass('foo', Mapping)`, I 
thought there might be a bug somewhere in my code. Then I looked deeper and 
found where this is really coming from.

I experimented more and noticed that `issubclass('foo', Reversible)` raises 
AttributeError, which isn't even the same type of error.

The exceptions that are raised in these cases seem like an abstraction leak. 
The error messages do not help users immediately realize what they did wrong 
and how they can fix it; more knowledge of internals is required to make sense 
of what's going on than should be needed. The inconsistency in these errors is 
a further problem. The same mistake should not cause three different errors 
unless there is some really good reason. This seems unintentional. Can any of 
the original authors say whether this is working as intended or if this is in 
fact an oversight?

The current behavior causes confusion for both less experienced and more 
experienced Python users alike. (Would anyone else here have correctly 
predicted all of the different errors that the examples above cause? How many 
other Python experts could have?) For less experienced users, Python's general 
consistency and predictability, lack of gotchas, and good errors are some of 
its best features. This is such an exception that it seems like a bug.

I'm happy for some other patch than the one I submitted in 
https://github.com/python/cpython/pull/5944 to land if necessary, as long as 
something fixes this. And fwiw, +1 for 3.7, unless anyone can demonstrate any 
credible risk.

Thanks for your consideration :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-10 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

I agree except that I'd like to see it in 3.7 too.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-10 Thread INADA Naoki

INADA Naoki  added the comment:

My current opinion is:

* -1 for 3.6:  Behavior should not be changed without strong reason, even the 
behavior is not documented.
* +1 for 3.8:  I like strict and less magic.
* +0 for 3.7:  beta3 is bit late, but this change has very little chance to 
cause real world problem.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-09 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

Regarding status quo (expanding the examples of @inada.naoki and @jab):

>>> import typing
>>> import collections.abc as cabc
>>> issubclass(typing.Mapping, cabc.Mapping)
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/izbyshev/workspace/cpython/Lib/abc.py", line 143, in 
__subclasscheck__
return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class
>>> from abc import ABC
>>> issubclass(typing.Mapping, ABC)
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/izbyshev/workspace/cpython/Lib/abc.py", line 143, in 
__subclasscheck__
return _abc_subclasscheck(cls, subclass)
  File "/home/izbyshev/workspace/cpython/Lib/abc.py", line 143, in 
__subclasscheck__
return _abc_subclasscheck(cls, subclass)
  File "/home/izbyshev/workspace/cpython/Lib/contextlib.py", line 30, in 
__subclasshook__
return _collections_abc._check_methods(C, "__enter__", "__exit__")
  File "/home/izbyshev/workspace/cpython/Lib/_collections_abc.py", line 73, in 
_check_methods
mro = C.__mro__
  File "/home/izbyshev/workspace/cpython/Lib/typing.py", line 706, in 
__getattr__
raise AttributeError(attr)
AttributeError: __mro__
>>> ABC.register(int)

>>> issubclass(typing.Mapping, ABC)
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/izbyshev/workspace/cpython/Lib/abc.py", line 143, in 
__subclasscheck__
return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class
>>> typing.Mapping.__mro__ = ()
>>> issubclass(typing.Mapping, ABC)
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/izbyshev/workspace/cpython/Lib/abc.py", line 143, in 
__subclasscheck__
return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class
>>> typing.Mapping.__bases__ = ()
>>> issubclass(typing.Mapping, ABC)
False

Can't say that I'm OK with it :) 

I'm for forbidding non-types in ABCMeta.__subclasscheck__, but if we are to add 
clean support for "class-likes" instead, I think that "class-like" objects 
should be clearly defined, for example, that they must have __mro__ and 
__bases__ (and probably support weakrefs unless we want to skip caching if they 
don't). ABCMeta.__subclasscheck__ is not standalone: it relies on issubclass() 
and __subclasshook__, and both of them have some expectations in practice.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-09 Thread Ivan Levkivskyi

Ivan Levkivskyi  added the comment:

To be honest I am still undecided on this. In principle, I am OK with status 
quo, but I am also OK, with the PR that will prohibit non-classes. I am a bit 
worried that it may break some existing code, so it is probably not for 3.7.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-09 Thread INADA Naoki

INADA Naoki  added the comment:

>> Isn't it just a limitation?
>> Most Python-implemented objects supports weakref. I don't think "requiring 
>> weakref support implies it must be type object".

> Formally, there is no implication. It is the abc module authors who know the 
> truth. But I can't imagine why anybody would impose such a limitation by 
> design, because while instances of user-defined classes support weakrefs, 
> built-in classes used by everybody like tuple, list and dict don't. That's 
> why I guessed that non-types were not meant to be supported.

Of course, issubclass(42, AnyABC) must raise TypeError.  They aren't class-like 
object. I didn't discuss on it.

I talked about class-like objects.
For example, this code works on Python 3.6, but not on 3.7.  typing.Mapping 
looks like type, but it is just an instance for 3.7.

  import typing
  import collections.abc as cabc
  print(issubclass(typing.MutableMapping, cabc.Mapping))  # Python 3.7 raises 
TypeError

I don't think it's real problem.  But if someone claims it's real issue, we can 
make typing.MutableMapping more "class-like" by adding __mro__.

diff --git a/Lib/typing.py b/Lib/typing.py
index 7ca080402e..2edaa3f868 100644
--- a/Lib/typing.py
+++ b/Lib/typing.py
@@ -619,6 +619,7 @@ def __init__(self, origin, params, *, inst=True, 
special=False, name=None):
   a for a in params)
 self.__parameters__ = _collect_type_vars(params)
 self.__slots__ = None  # This is not documented.
+self.__mro__ = (origin,) + getattr(origin, '__mro__', (object,))
 if not name:
 self.__module__ = origin.__module__


Again, I don't think it's a real problem.
Maybe, we can add the check, and revert it if someone claims.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-08 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

PR 5944 changes ABC.__subclasscheck__ (not issubclass) to check its first 
argument, so if it's merged there will be no crash even with the revert.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-08 Thread Serhiy Storchaka

Serhiy Storchaka  added the comment:

> Can 
> https://github.com/python/cpython/commit/fc7df0e664198cb05cafd972f190a18ca422989c
>  be reverted?

Even if subclass() will check explicitly that its first argument is a type, 
ABC.__subclasscheck__() can be called directly, and it shouldn't crash when 
called with non-type.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-08 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

> Isn't it just a limitation?
> Most Python-implemented objects supports weakref. I don't think "requiring 
> weakref support implies it must be type object".

Formally, there is no implication. It is the abc module authors who know the 
truth. But I can't imagine why anybody would impose such a limitation by 
design, because while instances of user-defined classes support weakrefs, 
built-in classes used by everybody like tuple, list and dict don't. That's why 
I guessed that non-types were not meant to be supported.

> What "by OP" means?
OP = Original poster (@jab).

> I can't find `if not issubclass(cls, type): raise TypeError` in Reversible 
> implementation.
> They do duck-typing, same to ABC.

Sorry for being unclear. There is no explicit check as you say, but __mro__ is 
directly accessed (see msg313376). But it may probably be considered "duck 
typing" too.

> But I don't know much about how mages use ABC.  I need mages comment before 
> merging the pull request.
Totally agree.

> BTW, do you think it should be backported to 3.7, or even 3.6?
3.7 certainly has my vote -- this can hardly be considered a new feature.

For 3.6, I'd listen to ABC users/experts. Might raising a TypeError instead of 
returning False from issubclass(user_defined_obj, ABC) break something 
important? Personally, I think it would mostly expose bugs and not hinder 
reasonable usage.  

> Can 
> https://github.com/python/cpython/commit/fc7df0e664198cb05cafd972f190a18ca422989c
>  be reverted?

Seems like it can, but the test should survive in some form :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-08 Thread INADA Naoki

INADA Naoki  added the comment:

> 1. ABCMeta.register() accepts types only.

Yes.  While ABC.register() and issubclass() have different users (e.g. 
ABC.register() will be used by  framework author, and issubclass will be used 
by framework users), it's positive reason to remove non-type support.

> 2. ABCMeta.__subclasscheck__ implicitly requires its arguments to support 
> weak references (regardless of whether __subclasshook__ is called or not). 
> This requirement alone doesn't make sense, so it seems to be an exposed 
> implementation detail stemming from the fact that non-types were not intended 
> to be supported.

Isn't it just a limitation?
Most Python-implemented objects supports weakref. I don't think "requiring 
weakref support implies it must be type object".


> 3. Some ABC users already expect that the argument of __subclasshook__ is a 
> type (see the example with collections.abc.Reversible by OP).

What "by OP" means?
I can't find `if not issubclass(cls, type): raise TypeError` in Reversible 
implementation.
They do duck-typing, same to ABC.


> 4. Attempting to support arbitrary arguments in ABC.__subclasscheck__ (by 
> returning False instead of raising TypeError or worse) will not solve any 
> 'issubclass' inconsistencies. 'issubclass' is fundamentally "fragmented": 
> issubclass(x, y) may return True/False while issubclass(x, z) may raise 
> TypeError, depending on __subclasscheck__ implementation.

Yes, as I commented above.

> It may be too late to impose stricter requirements for the first argument of 
> issubclass because 'typing' module relies on the support of non-types there.

Of course.


Personally speaking, I dislike magics including ABC, __subclasscheck__, 
overwriting __class__ with dummy object.  So I'm OK to limit the ability of it.

But I don't know much about how mages use ABC.  I need mages comment before 
merging the pull request.

BTW, do you think it should be backported to 3.7, or even 3.6?
Can 
https://github.com/python/cpython/commit/fc7df0e664198cb05cafd972f190a18ca422989c
 be reverted?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-08 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

I do not see any point in allowing non-types in ABCMeta.__subclasscheck__. 
Currently, ABCs are clearly not designed to support non-types:

1. ABCMeta.register() accepts types only.
2. ABCMeta.__subclasscheck__ implicitly requires its arguments to support weak 
references (regardless of whether __subclasshook__ is called or not). This 
requirement alone doesn't make sense, so it seems to be an exposed 
implementation detail stemming from the fact that non-types were not intended 
to be supported.
3. Some ABC users already expect that the argument of __subclasshook__ is a 
type (see the example with collections.abc.Reversible by OP).
4. Attempting to support arbitrary arguments in ABC.__subclasscheck__ (by 
returning False instead of raising TypeError or worse) will not solve any 
'issubclass' inconsistencies. 'issubclass' is fundamentally "fragmented": 
issubclass(x, y) may return True/False while issubclass(x, z) may raise 
TypeError, depending on __subclasscheck__ implementation. It may be too late to 
impose stricter requirements for the first argument of issubclass because 
'typing' module relies on the support of non-types there.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-07 Thread INADA Naoki

INADA Naoki  added the comment:

Hmm, normal class doesn't support issubclass(class-like. class).

```
Python 3.8.0a0 (heads/master:fc7df0e664, Mar  8 2018, 09:00:43)
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import typing
>>> issubclass(typing.MutableMapping, object)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: issubclass() arg 1 must be a class
>>> issubclass(typing.MutableMapping, type)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: issubclass() arg 1 must be a class
>>> issubclass(typing.MutableMapping, typing.Mapping)
True
```

OK, problem is ABC should support it or not.

--
nosy: +levkivskyi

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-07 Thread INADA Naoki

INADA Naoki  added the comment:

issubclass(class-like, class-like) is allowed.
I don't think raising type error for issubclass(class-like, ABC) is good idea.  
It should return False.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-07 Thread INADA Naoki

INADA Naoki  added the comment:

https://bugs.python.org/msg313396

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-07 Thread Alexey Izbyshev

Alexey Izbyshev  added the comment:

ABC.register() has an explicit check, and it is mentioned in PEP 3119. The 
point here is not to change issubclass(), but to change 
ABC.__subclasscheck__(). It may conceivably have stricter requirements than 
issubclass() has. But certainly an advice from actual ABC users would be nice.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-07 Thread INADA Naoki

INADA Naoki  added the comment:

Why `issubclass()` doesn't check it?  Maybe, non-type class is supported by 
Python.  But I'm not sure.  I'm not meta programming expert.

But we use "duck typing".  In this case, if the object (a) supports weakref and 
(2) has __mro__ and it is tuple, we treat the object as a class.

And when raising TypeError, information about which assumption was broken may 
be useful.

So I'm -1 on adding such strong check or removing internal information without 
meta programming expert advice.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33018] Improve issubclass() error checking and message

2018-03-06 Thread Joshua Bronson

New submission from Joshua Bronson :

Creating this issue by request of INADA Naoki to discuss my proposed patch in 
https://github.com/python/cpython/pull/5944.

Copy/pasting from that PR:

If you try something like issubclass('not a class', str), you get a helpful 
error message that immediately clues you in on what you did wrong:

>>> issubclass('not a class', str)
TypeError: issubclass() arg 1 must be a class
("AHA! I meant isinstance there. Thanks, friendly error message!")

But if you try this with some ABC, the error message is much less friendly!

>>> from some_library import SomeAbc
>>> issubclass('not a class', SomeAbc)
Traceback (most recent call last):
  File "", line 1, in 
  File 
"/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/abc.py",
 line 230, in __subclasscheck__
cls._abc_negative_cache.add(subclass)
  File 
"/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/_weakrefset.py",
 line 84, in add
self.data.add(ref(item, self._remove))
TypeError: cannot create weak reference to 'str' object

("WTF just went wrong?" Several more minutes of head-scratching ensues. Maybe a 
less experienced Python programmer who hits this hasn't seen weakrefs before 
and gets overwhelmed, maybe needlessly proceeding down a deep rabbit hole 
before realizing no knowledge of weakrefs was required to understand what they 
did wrong.)

Or how about this example:

>>> from collections import Reversible
>>> issubclass([1, 2, 3], Reversible)
Traceback (most recent call last):
  File "", line 1, in 
  File 
"/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/abc.py",
 line 207, in __subclasscheck__
ok = cls.__subclasshook__(subclass)
  File 
"/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/_collections_abc.py",
 line 305, in __subclasshook__
return _check_methods(C, "__reversed__", "__iter__")
  File 
"/usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/_collections_abc.py",
 line 73, in _check_methods
mro = C.__mro__
AttributeError: 'list' object has no attribute '__mro__'
Here you don't even get the same type of error (AttributeError rather than 
TypeError), which seems unintentionally inconsistent.

This trivial patch fixes this, and will hopefully save untold numbers of future 
Python programmers some time and headache.

Let me know if any further changes are required, and thanks in advance for 
reviewing.

--
messages: 313376
nosy: inada.naoki, izbyshev, jab, serhiy.storchaka
priority: normal
pull_requests: 5781
severity: normal
status: open
title: Improve issubclass() error checking and message
type: enhancement
versions: Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com