Smart_open: S3OpenRead doesn't work with TextIOWrapper

Created on 7 Mar 2016 · 7Comments · Source: RaRe-Technologies/smart_open

Currently only binary-mode read is supported, which would be OK if the stream you returned worked with TextIOWrapper -- but it doesn't, at least not S3OpenRead. TextIOWrapper expects readable, writable, seekable, closed etc methods. If you make S3OpenRead inherit (or quack like) IOBase that should fix it.

enhancement hard wishlist

Source

mjwillson

👍3

Most helpful comment

Hey, sorry I'm unlikely to get around to this as it stands -- we've worked around it like so, although this won't work for everyone:

            # smart_open's S3OpenRead doesn't support TextIOWrapper
            # and smart_open doesn't support text-mode open. So
            # we return a poor-man's line-by-line TextIOWrapper:
            return (str(line, "utf-8") for line in text_stream)

To be honest smart_open feels like quite a leaky abstraction due to patchy / partial support for various features, meaning you need extra special-cased dispatch and wrapping code after parsing the URL yourself. E.g. gzip only working for local files, some other common compression formats not supported, text mode not working for S3, https issues. Maybe some of those are fixed now, but if I was to revisit I'd be tempted to just manually wrap the streams from the underlying libraries myself.

If you wanted to address that in a deeper way, I'd suggest making sure any stream implementations here respect the stream interfaces from the standard library ( https://docs.python.org/3/library/io.html ), which would make it easier to write general and composable code that builds on them. Perhaps some of that would be easier if you give up on Python 2. Then try and implement features like compression and text mode support in a general way where possible via wrapping streams, and make the dispatch mechanisms (for protocols, compression formats etc) extensible.

mjwillson on 5 Oct 2016

👍3

All 7 comments

I'm not familiar with hacking these classes. If you send a PR, we can add that support.

Is it possible to add it globally to all readers? (not just S3OpenRead)

piskvorky on 8 Mar 2016

Ping @mjwillson

tmylk on 5 Oct 2016

Hey, sorry I'm unlikely to get around to this as it stands -- we've worked around it like so, although this won't work for everyone:

            # smart_open's S3OpenRead doesn't support TextIOWrapper
            # and smart_open doesn't support text-mode open. So
            # we return a poor-man's line-by-line TextIOWrapper:
            return (str(line, "utf-8") for line in text_stream)

mjwillson on 5 Oct 2016

👍3

Thanks for the idea of stream interfaces. Leaving open as it is an interesting direction to explore.

tmylk on 5 Oct 2016

This is related to #91. If we make our S3 objects behave like the base classes from the io package, this issue will be resolved.