Skip to content

bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ #16664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Oct 8, 2019

I 've updated to codecs.IncrementalDecoder to be able to get True from below code snippets.

>>> issubclass(_pyio.IncrementalNewlineDecoder, codecs.IncrementalDecoder)
True
>>> issubclass(_io.IncrementalNewlineDecoder, codecs.IncrementalDecoder)
True
>>> issubclass(io.IncrementalNewlineDecoder, codecs.IncrementalDecoder)
True
>>> class A(_io.IncrementalNewlineDecoder):
...     pass
...
>>> class B(_pyio.IncrementalNewlineDecoder):
...     pass
...
>>> issubclass(A, codecs.IncrementalDecoder)
True
>>> issubclass(B, codecs.IncrementalDecoder)
True

https://bugs.python.org/issue31722

@corona10
Copy link
Member Author

corona10 commented Oct 8, 2019

@serhiy-storchaka @vstinner

For reviewers,
IMHO, The more straightforward way to solve this issue is set tp_base of _io.IncrementalNewlineDecoder to codecs.IncrementalDecoder.
But I can't find a way to this on C API level to work with pure python object type.

So my choice is to use ABC.abc.
Please let me know if I am going in the wrong way.

Thanks always!

@corona10 corona10 added the type-bug An unexpected behavior, bug, or error label Oct 8, 2019
@corona10 corona10 changed the title bpo-31722: Fix io.IncrementalNewlineDecoder to be subclass of codecs.IncrementalDecoder bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ Oct 8, 2019
@corona10
Copy link
Member Author

corona10 commented Oct 9, 2019

Or How about switching codecs.IncrementalDecoder into the C implementation _codecs.IncrementalDecoderand then
inherit it on the _pyio. IncrementalNewlineDecoder and _io. IncrementalNewlineDecoder ?

@corona10 corona10 changed the title bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ [WIP] bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ Oct 9, 2019
@serhiy-storchaka
Copy link
Member

The problem is not that issubclass() returns False, but that two classes are not truly compatible.

@corona10 corona10 changed the title [WIP] bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ bpo-31722: Update codecs.IncrementalDecoder to use __subclasshook__ Oct 9, 2019
@serhiy-storchaka
Copy link
Member

Implementing IncrementalDecoder in C could work, but it would add more complexity. I do not know what is the best solution of this issue.

@corona10
Copy link
Member Author

corona10 commented Oct 9, 2019

@serhiy-storchaka

Yes, I tried it today and I found that it is a complex issue better than I expected.
Since this class is used in the below pattern.

class IncrementalDecoder(mbc.MultibyteIncrementalDecoder,
                                              codecs.IncrementalDecoder):

We should implement it to be able to avoid lay-out conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants