Let PipedCompressionWriter/-Reader derive from IOBase #140
Conversation
rhpvorderman
left a comment
rhpvorderman
left a comment
There was a problem hiding this comment.
Nice, this has been on the wishlist a long time!
| return self | ||
|
|
||
| def __next__(self) -> Union[str, bytes]: | ||
| def __next__(self) -> Union[str, bytes]: # type: ignore # incompatible with type in IOBase |
There was a problem hiding this comment.
This is something I ran into during my refactoring in #138 . The externally opened classes are slightly different in that they both support t and b modes. It would be much more in line with the rest of python if they are binary only, and a textwrapper is used around the pipedcompressionreader/writer objects. (Rather than interning a TextIOWrapper in the object as is done now). This prevented me from simplifying the gzip opening further.
But that is something for later. I will make an issue about it.
There was a problem hiding this comment.
TextIOWrapper derives from TextIOBase, which derives from IOBase, so the problem that the return type changes in a derived class is still there. typeshed "solved" the issue the same way:
https://github.com/python/typeshed/blob/770724013de34af6f75fa444cdbb76d187b41875/stdlib/io.pyi#L145
I read a comment somewhere on the typeshed issue tracker stating that Python’s IO classes are just weird, and I agree.
It’s true we won’t have to deal with this when we make the Piped... classes work with binary data only. I agree and like the idea.
There was a problem hiding this comment.
Good to know that the issue is not something that is necessarily wrong on xopen's end. Thank you for the explanation.
This is the proper thing to do and also gives us a couple of methods for free, in particular readlines(). This also allows us to get rid of the Closing mixin. Closes #129
|
Looks good to me! |
This is (probably) the proper thing to do for custom IO classes and also gives us a couple of methods for free, in particular
readlines().This allows us to get rid of the
Closingmixin (IOBase gives us the same functionality).I had to also fix some warnings (due to
PYTHONDEVMODE=1) caused by some objects being in a half-initialized stage whenclose()is called.Closes #129