classification
Title: os.PathLike subclasshook causes subclass checks true on abstract implementation
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ammar2, bar.harel, levkivskyi, xtreak
Priority: normal Keywords: patch

Created on 2019-11-21 15:00 by bar.harel, last changed 2019-12-23 18:32 by levkivskyi. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17336 merged bar.harel, 2019-11-22 10:23
PR 17684 merged bar.harel, 2019-12-23 13:14
PR 17685 merged bar.harel, 2019-12-23 13:18
Messages (13)
msg357174 - (view) Author: Bar Harel (bar.harel) * Date: 2019-11-21 15:00
Quick and small fix.

os.PathLike.__subclasshook__ does not check if cls is PathLike as abstract classes should.

This in turn causes this bug:

    class A(PathLike):
        pass

    class B:
        def __fspath__(self):
            pass

    assert issubclass(B, A)

I will fix the bug later today and push a patch over to python/cpython on GitHub.
msg357205 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-21 19:45
I can't reproduce in Python 3.8.0:

>>> import os
>>> class A(os.PathLike): pass
...
>>> class B:
...     def __fspath__(self): pass
...
>>> issubclass(B, A)
True

Did you check against an older version of Python?
msg357209 - (view) Author: Bar Harel (bar.harel) * Date: 2019-11-21 20:00
Hey Brett, that's exactly the bug. It's supposed to be False ofc.

On Thu, Nov 21, 2019, 9:45 PM Brett Cannon <report@bugs.python.org> wrote:

>
> Brett Cannon <brett@python.org> added the comment:
>
> I can't reproduce in Python 3.8.0:
>
> >>> import os
> >>> class A(os.PathLike): pass
> ...
> >>> class B:
> ...     def __fspath__(self): pass
> ...
> >>> issubclass(B, A)
> True
>
> Did you check against an older version of Python?
>
> ----------
> resolution:  -> not a bug
> stage:  -> resolved
> status: open -> closed
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue38878>
> _______________________________________
>
msg357213 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-21 20:16
Ah, your `assert` call threw me since it does succeed so it isn't acting as a test case.
msg357326 - (view) Author: Bar Harel (bar.harel) * Date: 2019-11-22 21:54
Done.

On Fri, Nov 22, 2019, 12:23 PM Bar Harel <report@bugs.python.org> wrote:

>
> Change by Bar Harel <bzvi7919@gmail.com>:
>
>
> ----------
> keywords: +patch
> pull_requests: +16820
> stage: resolved -> patch review
> pull_request: https://github.com/python/cpython/pull/17336
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue38878>
> _______________________________________
>
msg357342 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-22 23:38
I just realized one problem with this is it explicitly requires subclassing the ABC while os.PathLike is supposed to represent a protocol (before typing.Protocol was a thing).

So why is it bad that in the example class B is considered a "subclass" of os.PathLike by implementing the protocol? Since it implements the expected protocol what exactly is being lost by not checking for an explicit registration or subclass?
msg357354 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-11-23 01:19
> So why is it bad that in the example class B is considered a "subclass" of os.PathLike by implementing the protocol?

This is not bad, my understanding of the problem is that currently B is considered a subclass of A, while the latter should not be structural.

To give an analogy with PEP 544 (sorry, this is my favourite one :-)) consider this:

class P(Protocol):
    def some(self): ...

class C:
    def some(self): ...

Here C is obviously a "subclass" of P, but:

class Bad(P):  # <- this is _no_ a protocol, just a nominal class
    pass       # explicitly subclassing P

class Good(P, Protocol):  # <- this is a subprotocol that
    pass                  # happened to be identical to P

So here C is a "subclass" of Good, but not a "subclass" of Bad.
msg357356 - (view) Author: Ammar Askar (ammar2) * (Python triager) Date: 2019-11-23 01:39
Just for reference/existing behavior:

>>> class A(collections.abc.Iterable): pass
...
>>> class B:
...   def __iter__(): pass
...
>>> issubclass(B, A)
False
>>> issubclass(B, collections.abc.Iterable)
True
>>> issubclass(A, collections.abc.Iterable)
True
msg357367 - (view) Author: Bar Harel (bar.harel) * Date: 2019-11-23 09:29
A better example is this:

    class A(PathLike):
        def foo(self):
            """For all your foo needs"""

    class B:
        def __fspath__(self):
            pass

    issubclass(B, A) == True
    A().foo() # Yay, I Foo'd.
    B().foo() # oh barnacles, I should have stayed at home.
msg357776 - (view) Author: Bar Harel (bar.harel) * Date: 2019-12-04 06:36
Ready for merge
msg358787 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-12-22 09:57
New changeset eae87e3e4e0cb9a0ce10c2e101acb6995d79e09c by Ivan Levkivskyi (Bar Harel) in branch 'master':
bpo-38878: Fix os.PathLike __subclasshook__ (GH-17336)
https://github.com/python/cpython/commit/eae87e3e4e0cb9a0ce10c2e101acb6995d79e09c
msg358828 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-12-23 18:31
New changeset 0846e5d4603434c2bbf8a528677cf1ff3fe29b95 by Ivan Levkivskyi (Bar Harel) in branch '3.8':
[3.8] bpo-38878: Fix os.PathLike __subclasshook__ (GH-17336) (GH-17684)
https://github.com/python/cpython/commit/0846e5d4603434c2bbf8a528677cf1ff3fe29b95
msg358829 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-12-23 18:31
New changeset 59d06b987db34cde8783e265709366d244c9e35b by Ivan Levkivskyi (Bar Harel) in branch '3.7':
[3.7] bpo-38878: Fix os.PathLike __subclasshook__ (GH-17336) (GH-17685)
https://github.com/python/cpython/commit/59d06b987db34cde8783e265709366d244c9e35b
History
Date User Action Args
2019-12-23 18:32:01levkivskyisetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: - Python 3.6
2019-12-23 18:31:18levkivskyisetmessages: + msg358829
2019-12-23 18:31:08levkivskyisetmessages: + msg358828
2019-12-23 13:18:34bar.harelsetpull_requests: + pull_request17142
2019-12-23 13:14:46bar.harelsetpull_requests: + pull_request17141
2019-12-22 09:57:57levkivskyisetmessages: + msg358787
2019-12-04 20:11:42brett.cannonsetnosy: - brett.cannon
2019-12-04 06:36:29bar.harelsetmessages: + msg357776
2019-11-23 09:29:43bar.harelsetmessages: + msg357367
2019-11-23 06:41:09xtreaksetnosy: + xtreak
2019-11-23 01:39:00ammar2setnosy: + ammar2
messages: + msg357356
2019-11-23 01:19:53levkivskyisetmessages: + msg357354
2019-11-22 23:38:20brett.cannonsetnosy: + levkivskyi
messages: + msg357342
2019-11-22 21:54:48bar.harelsetmessages: + msg357326
2019-11-22 10:23:05bar.harelsetkeywords: + patch
stage: resolved -> patch review
pull_requests: + pull_request16820
2019-11-21 20:16:36brett.cannonsetstatus: closed -> open
resolution: not a bug -> (no value)
messages: + msg357213
2019-11-21 20:00:51bar.harelsetmessages: + msg357209
2019-11-21 19:45:15brett.cannonsetstatus: open -> closed
resolution: not a bug
messages: + msg357205

stage: resolved
2019-11-21 15:22:22BTaskayasetnosy: + brett.cannon
2019-11-21 15:00:21bar.harelcreate