Smart_open: Line splitting on \u2028 from S3

[chrisfleisch]

Problem description

Be sure your description clearly answers the following questions:

• What are you trying to achieve?

Reading lines of text with a character \u2028 from S3 produces different output than from the local disk. I would like the output from S3 to match what happens from local disk.

• What is the expected result?

Lines of text from the local disk are not split on a \u2028 character in a string of text.

• What are you seeing instead?

Lines of text from the S3 are split on a \u2028 character in a string of text.

Steps/code to reproduce the problem

In order for us to be able to solve your problem, we have to be able to reproduce it on our end.
Without reproducing the problem, it is unlikely that we'll be able to help you.

Include full tracebacks, logs and datasets if necessary.
Please keep the examples minimal (minimal reproducible example).

# create test file
print('C123 Overview:\u2028 Earl', file=Path('/aws_home/bad_split_text.txt').open('w'))
# then upload to bad_split_text.txt to S3

c1 = [l for l in smart_open.open('/aws_home/bad_split_text.txt', encoding='utf-8', newline='\n')]
c2 = [l for l in smart_open.open('s3://mybucket/bad_split_text.txt', encoding='utf-8', newline='\n')]

print(repr(c1)) # ['C123 Overview:\u2028 Earl\n']
print(repr(c2)) # ['C123 Overview:\u2028', ' Earl\n']

Versions

Please provide the output of:

import platform, sys, smart_open
print(platform.platform())
print("Python", sys.version)
print("smart_open", smart_open.__version__)

Linux-5.4.0-1029-aws-x86_64-with-glibc2.10
Python 3.8.5 | packaged by conda-forge | (default, Sep 24 2020, 16:55:52)
[GCC 7.5.0]
smart_open 3.0.0

Checklist

Before you create the issue, please make sure you have:

• [x] Described the problem clearly
• [x] Provided a minimal reproducible example, including any required data
• [x] Provided the version numbers of the relevant software

[mpenkov]

U+2028 is a special character that indicates the end of a line.

If you want to avoid splitting around it, read the file as binary, iterate over the binary lines, and then decode each binary line separately, e.g.

for bline in smart_open.open(path, 'rb') as fin:
    line = bline.decode('utf-8')
    ...

That said, the difference in behavior between local and remote reading _does_ sound like a bug.

[chrisfleisch]

Thanks. When reading as binary for the remote I can now get the same results as the local file.

[markopy]

I have encountered this same problem using local files with a custom decompression handler. This is an actual bug, U+2028 should not split lines. The python built in open() never does this which is the reason it works correctly only when opening local files. All other transports or using decompressors is broken.

The problem is in using codecs to wrap text files. It seems to split lines when encountering the unicode line separator. The only way to turn this off seems to be passing it encoding = None but that's not possible with smart_open because DEFAULT_ENCODING is substituted in that case.

I don't think it's possible to get this working correctly while using the codecs module. For python3 the best approach might be to somehow use io.TextIOWrapper instead. That way newline can also be supported correctly. Right now it's completely ignored for anything except opening local files.

I believe issue #536 is also caused by this.

[markopy]

I found some discussion of this at https://bugs.python.org/issue12855 which points to https://docs.python.org/3/library/stdtypes.html#str.splitlines for the list of code points that codecs treats as newlines:

Representation | Description
-- | --
\n | Line Feed
\r | Carriage Return
\r\n | Carriage Return + Line Feed
\v or \x0b | Line Tabulation
\f or \x0c | Form Feed
\x1c | File Separator
\x1d | Group Separator
\x1e | Record Separator
\x85 | Next Line (C1 Control Code)
\u2028 | Line Separator
\u2029 | Paragraph Separator

It also confirms my suspicion that the codecs module can not be used to implement this properly. However io.TextIOWrapper seems to be available in python 2.7 as well so I would suggest just replacing

    if encoding is None:
        encoding = DEFAULT_ENCODING

    kw = {'errors': errors} if errors else {}
    if mode[0] == 'r' or mode.endswith('+'):
        fileobj = codecs.getreader(encoding)(fileobj, **kw)
    if mode[0] in ('w', 'a') or mode.endswith('+'):
        fileobj = codecs.getwriter(encoding)(fileobj, **kw)

with

    if encoding is None:
        encoding = DEFAULT_ENCODING
    fileobj = io.TextIOWrapper(fileobj, encoding=encoding, errors=errors, newline=newline)

in _encoding_wrapper()

[markopy]

More issues with the same problem:

394 - Incorrect line separation for compressed files

269 - Actually identifies the problem correctly as being codecs.getreader()

Note that there are also issues like #493 where mode='rt' being passed to gzip causes issues. No consideration seems to be given in the code as to whether underlying file objects should be opened in binary mode if smart_open does apply a codec. There could be subtle issues with double encoding/decoding.

[mpenkov]

Thank you for the detailed information. Are there any performance benefits/costs in moving from codecs to your proposed method? Are you interested in making a PR?

[markopy]

@mpenkov There seems to be no performance difference. TextIOWrapper actually uses codecs internally so that makes sense.