Skip to content
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

str.splitlines splitting on non-\r\n characters #66428

Open
scharron mannequin opened this issue Aug 20, 2014 · 34 comments
Open

str.splitlines splitting on non-\r\n characters #66428

scharron mannequin opened this issue Aug 20, 2014 · 34 comments
Labels
docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@scharron
Copy link
Mannequin

scharron mannequin commented Aug 20, 2014

BPO 22232
Nosy @malemburg, @warsaw, @nascheme, @terryjreedy, @gpshead, @vstinner, @jwilk, @ezio-melotti, @stevendaprano, @bitdancer, @vadmium, @serhiy-storchaka
Files
  • cpython3.5_splitlines.diff
  • cpython3.5_splitlines.diff
  • cpython2.7_splitlines.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2014-08-20.10:01:51.226>
    labels = ['type-bug', 'docs']
    title = 'str.splitlines splitting on non-\\r\\n characters'
    updated_at = <Date 2018-10-07.20:48:13.606>
    user = 'https://bugs.python.org/scharron'

    bugs.python.org fields:

    activity = <Date 2018-10-07.20:48:13.606>
    actor = 'nascheme'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2014-08-20.10:01:51.226>
    creator = 'scharron'
    dependencies = []
    files = ['43071', '43072', '43075']
    hgrepos = []
    issue_num = 22232
    keywords = ['patch']
    message_count = 34.0
    messages = ['225561', '225564', '225705', '225709', '225723', '225736', '225747', '225748', '225755', '225758', '225766', '225767', '225769', '225873', '225874', '230138', '230143', '230144', '230152', '246547', '246570', '266781', '266782', '266785', '266789', '266800', '327105', '327106', '327113', '327137', '327141', '327162', '327269', '327308']
    nosy_count = 16.0
    nosy_names = ['lemburg', 'barry', 'nascheme', 'terry.reedy', 'gregory.p.smith', 'vstinner', 'jwilk', 'ezio.melotti', 'steven.daprano', 'r.david.murray', 'docs@python', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'scharron', 'Alexander Schrijver']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22232'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @scharron
    Copy link
    Mannequin Author

    scharron mannequin commented Aug 20, 2014

    According to the documentation, str.splitlines uses the universal newlines to split lines.
    The documentation says it's all about \r, \n, and \r\n (https://docs.python.org/3.5/glossary.html#term-universal-newlines)

    However, it's also splitting on other characters. Reading the code, it seems the list of characters is from Objects/unicodeobject.c , in _PyUnicode_Init, the linebreak array.
    When testing any of these characters, it splits the string.

    Other libraries are using str.splitlines assuming it only breaks on these \r and \n characters. This is the case of email.feedparser for instance, used by http.client to parse headers. These HTTP headers should be separated by CLRF as specified by http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.

    Either the documentation should state that splitlines splits on other characters or it should stick to the documentation and split only on \r and \n characters.

    If it splits on other characters, the list could be improved, as the unicode reference lists the mandatory characters for line breaking : http://www.unicode.org/reports/tr14/tr14-32.html#BK

    @scharron scharron mannequin added stdlib Python modules in the Lib dir topic-unicode type-bug An unexpected behavior, bug, or error labels Aug 20, 2014
    @scharron
    Copy link
    Mannequin Author

    scharron mannequin commented Aug 20, 2014

    For an example of a serious bug caused by this, see http://bugs.python.org/issue22233

    @terryjreedy
    Copy link
    Member

    Objects/unicodeobject.c linebreak is at 266. With 3.4.1:
    >>> 'a\x0ab\x0bc\x0cd\x0d1c\x1c1d\x1d1e\x1e'.splitlines()
    ['a', 'b', 'c', 'd', '1c', '1d', '1e']
    \x0a == \n, \x0d == \r
    
    The \r\n pair is a special case, as promised, but other pairs are not.
    >>> 'a\r\nb'.splitlines()
    ['a', 'b']
    >>> 'a\x0b\nb'.splitlines()
    ['a', '', 'b']

    @terryjreedy terryjreedy changed the title str.splitlines splitting on none-\r\n characters str.splitlines splitting on non-\r\n characters Aug 22, 2014
    @scharron
    Copy link
    Mannequin Author

    scharron mannequin commented Aug 22, 2014

    It's also at line bpo-14941 for unicode strings if I understand correctly

    With 3.4.0:

    >>> "a\x85b\x1ec".splitlines()
    ['a', 'b', 'c']

    @bitdancer
    Copy link
    Member

    See bpo-7643 for some technical background. There are some other interesting issues to read if you seach the tracker for 'splitlines unicode', one of which is an open doc issue. Clearly the docs about this are inadequate.

    Basically, though, I think you are correct. email should not be using splitlines(). It was more or less correct when email was splitting binary data, but even then it wasn't exactly correct per the letter of RFC.

    Unfortunately not using splitlines has some performance implications...but then again we haven't done any sort of performance improvement pass on the new email code, so it may well be marginal in the overall scheme of things.

    @scharron
    Copy link
    Mannequin Author

    scharron mannequin commented Aug 23, 2014

    This is a known issue, and will be resolved by improving documentation, I'm closing this bug

    Thanks !

    @scharron scharron mannequin closed this as completed Aug 23, 2014
    @bitdancer
    Copy link
    Member

    OK, we'll use bpo-22232 to resolve the issue of email using splitlines.

    @serhiy-storchaka
    Copy link
    Member

    May be add a parameter to str.splitlines() which will switch behavior to split on '\n', '\r' and '\r\n' only?

    @terryjreedy
    Copy link
    Member

    Unless there is already another issue for improving the doc, this should at least be left open as a doc issue.

    But I had the same thought as Serhiy, that we should at least optionally make the current doc correct. Two possibilities:

    newlines=False  If true, only split on \r, \n, \r\n; otherwise split on all latin-1 linebreak characters -- <list>.  {This is rather awkward.}
    
    linebreak=True  If true, split on all latin-1 linebreak characters <list>; otherwise only split on \r, \n, \r\n.  {Better, to me}

    Changing both code and doc, at least in 3.5, says that both are wrong. If we agree on this, there is still the awkward issue of what to do for 3.4. Just change the doc? Then email must do something different in 3.4 to work around the code behavior. I think this may warrant a pydev discussion.

    Another issue is whether latin-1 linebreaks are privileged. Why not implement the full unicode linebreak algorithm.

    An additional complication is that in 2.x, .splitlines acts as advertised.

    >>> 'a\x0ab\x0bc\x0cd\x0dda\x0d\x0a1c\x1c1d\x1d1e\x1e85\x85end'.splitlines()
    ['a', 'b\x0bc\x0cd', 'da', '1c\x1c1d\x1d1e\x1e85\x85end']

    @terryjreedy terryjreedy reopened this Aug 23, 2014
    @serhiy-storchaka
    Copy link
    Member

    I don't understand why you say about latin-1. splitlines() supports linebreaks outside latin-1 range.

    >>> [hex(i) for i in range(sys.maxunicode + 1) if len(('%cx' % i).splitlines()) == 2]
    ['0xa', '0xb', '0xc', '0xd', '0x1c', '0x1d', '0x1e', '0x85', '0x2028', '0x2029']

    "newlines" and "linebreak" don't look good to me. And it is not obvious why true or false value corresponds to one or another variant.

    @terryjreedy
    Copy link
    Member

    I was not aware of the remainder of the undocumented behavior. Thanks for the code that makes it clear .

    linebreak (or linebreaks)=True means that splitting occurs on some (approximation?*) of unicode mandatory linebreaks, as opposed to just the ascii 'universal newline' sequences, as defined in our glossary. Possible alternative: restrict=False (restrict to u. newlines?)

    *I did not read the annex in enough detail to know either way.

    The following pair of experiments, which I should have run before, show that there has been no real change of behavior from 2.x to 3.x.
    # 2.7.8
    >>> u'a\x0ab\x0bc\x0cd\x0dda\x0d\x0a1c\x1c1d\x1d1e\x1e85\x852028\u20282029\u2029end'.splitlines()
    [u'a', u'b', u'c', u'd', u'da', u'1c', u'1d', u'1e', u'85', u'2028', u'2029', u'end']
    # 3.4.1
    b'a\x0ab\x0bc\x0cd\x0dda\x0d\x0a1c\x1c1d\x1d1e\x1e85\x85end'.splitlines()
    [b'a', b'b\x0bc\x0cd', b'da', b'1c\x1c1d\x1d1e\x1e85\x85end']

    Given this, I am a bit dubious about adding a new parameter in 3.5 to make the unicode method act like the bytes method. Part of my support for that was thinking that it would help porting code. But that is not true. In both 2 and 3, there is the possibility to latin-1 encode, split, and latin-1 decode the pieces.

    The doc correction clearly needed is that the 3.4+ universal newlines glossary entry needs to be updated from 'str.splitlines' to 'bytes.newlines'. I will try to do this.

    A second doc problem is that the docstrings (given by help(x.splitlines) are exactly the same for bytes.splitlines and unicode.splitlines, in both 2.x and 3.x, even though the behavior is different even for ascii. I think each should list what they split on. Ditto for the doc is not already. This should have a patch posted for review.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 23, 2014

    New changeset 3ad59ed0f4f0 by Terry Jan Reedy in branch '3.4':
    Issue bpo-22232 (partial fix): update Universal newlines Glossary entry.
    http://hg.python.org/cpython/rev/3ad59ed0f4f0

    @terryjreedy
    Copy link
    Member

    Glossary fixed. I changed the components to Documention as you will handle email elsewhere.

    For library references: The key sentence currently used in all entries is "This method uses the universal newlines approach to splitting lines.", where *universal newlines* is linked to the glossary.

    2.x has one entry for str and unicode. I propose to add "Unicode.splitlines also splits on '\x0b' ('\v'), '\x0c' ('\f'), '\x1c', '\x1d', '\x1e', '\x85', '\u2028', and '\u2029'."

    3.x bytes entry is good as is.

    3.x str entry is wrong. Replace with "This method splits on universal newlines and also on '\x0b' ('\v'), '\x0c' ('\f'), '\x1c', '\x1d', '\x1e', '\x85', '\u2028', and '\u2029'."

    The docstrings now contain about the same as the docs, minus the key line above.
    " Return a list of the lines in S, breaking at line boundaries.
    Line breaks are not included in the resulting list unless keepends
    is given and true."

    Between the sentences, I propose to add:
    "Boundaries are indicated by 'universal newlines' ('\x0a' ('\n'), '\x0d' ('\r'), and '\x0d\x0a' ('\r\n'))." for bytes,
    with the addition of "and '\x0b' ('\v'), '\x0c' ('\f'), '\x1c', '\x1d', '\x1e', '\x85', '\u2028', and '\u2029'" for unicode.

    @terryjreedy terryjreedy added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir topic-unicode topic-email labels Aug 23, 2014
    @bitdancer
    Copy link
    Member

    The existing related open doc issue bpo-12855.

    @bitdancer
    Copy link
    Member

    Ideally str.splitlines would split on whatever the unicode database says are mandatory line break characters. I take it this is currently not true? That is, that the list is hardcoded?

    @ezio-melotti
    Copy link
    Member

    Looks like str.splitlines is using STRINGLIB_ISLINEBREAK which in turn uses Py_UNICODE_ISLINEBREAK, so the behavior should be correct. If splitting on \n, \r, and \r\n only is common enough with might add a bool arg to splitlines to restrict the splitting on those 3 only, but I can't think about any good name for such arg.

    @serhiy-storchaka
    Copy link
    Member

    With Terry's explanation "linebreak" looks better to me. Yet one alternative is ascii=False (or unicode=True?). And may be worth to add this parameter to strip/rstrip/lstrip/split too. On other hand regular expressions can be used in such special cases.

    @ezio-melotti
    Copy link
    Member

    There are some ascii line breaks other than \n, \r, \r\n.
    unicode=True might be better, but might be confused with unicode strings.
    Maybe unicode_linebreaks or unicode_newlines?

    @serhiy-storchaka
    Copy link
    Member

    See also bpo-18236.

    @vadmium
    Copy link
    Member

    vadmium commented Jul 10, 2015

    The main documentation has been updated and bpo-12855 has been closed. What is left to do here, considering this is marked as a documenation bug? Just modify the doc strings, as Terry suggested in <https://bugs.python.org/issue22232#msg225766\>?

    @gpshead
    Copy link
    Member

    gpshead commented Jul 10, 2015

    If this isn't already mentioned in 2 to 3 porting notes it is worth highlighting there. code which uses a str in python 2 and still uses a str in python 3 is now splitting on many more characters.

    That seems to be the source of bugs like bpo-22233. splitlines() used to work for the strict \r\n splitting task. now that code needs to made explicit about its splitting desires.

    @AlexanderSchrijver
    Copy link
    Mannequin

    AlexanderSchrijver mannequin commented May 31, 2016

    This diff updates the cpython (tip) documentation to document the different behaviour when using splitlines on bytes objects or string objects.

    @AlexanderSchrijver
    Copy link
    Mannequin

    AlexanderSchrijver mannequin commented May 31, 2016

    This diff synchronizes the cpython 2.7 with that from 3.5 and also describes the difference between bytes objects and unicode objects (from the other diff)

    @AlexanderSchrijver
    Copy link
    Mannequin

    AlexanderSchrijver mannequin commented May 31, 2016

    Oops, wrong diff. Sorry, this is the correct one for 2.7.

    @vadmium
    Copy link
    Member

    vadmium commented May 31, 2016

    For Python 3, the bytes.splitlines() and bytearray.splitlines() documentation has been moved to a separate section out (bpo-21777). I don’t think it is good to add much detail of bytes.splitlines() in the str.splitlines() documentation.

    For Python 2, perhaps see Matthew’s patches for 2.7 in bpo-12855. IMO we could reopen that bug if that helps, because only the Python 3 branches were comitted.

    @AlexanderSchrijver
    Copy link
    Mannequin

    AlexanderSchrijver mannequin commented Jun 1, 2016

    I appeared to have missed the reference to that issue when I read this issue the first time. Re-opening that issue makes sense to me.

    @nascheme
    Copy link
    Member

    nascheme commented Oct 5, 2018

    If we introduce a keyword parameter, I think the default of str.splitlines() should be changed to match bytes.splitlines (and match Python 2 str.splitlines()). I.e. split on \r and \n by default. I looked through the stdline and I can't find any calls that should actually by splitting on the extra characters. I will check it again though.

    Does anyone have an example of where the current behaviour is actually wanted?

    @serhiy-storchaka
    Copy link
    Member

    If change the default behavior we need to wait several releases after adding this option. Users should be able to pick the current behavior explicitly.

    Currently the workaround is using regular expressions.

    For s.splitlines(keepends=False):

        re.split(r'\n|\r\n?', s)

    For s.splitlines(keepends=True):

    re.split(r'(?<=\n)|(?<=\r)(?!\n)', s)
    

    @malemburg
    Copy link
    Member

    I am -1 on changing the default behavior. The Unicode standard defines what a linebreak code point is (all code points with character properties Zl or bidirectional property B) and we adhere to that. This may confuse parsers coming from the ASCII world, but that's really a problem with those parsers assuming that .splitlines() only splits on ASCII line breaks, i.e. they are not written in a Unicode compatible way.

    As mentioned in https://bugs.python.org/issue18291 we could add a parameter to .splitlines(), but this would render the method not much faster than re.split().

    Using re.split() is not a work-around in his case, it's an explicit form of defining the character you want to split lines on, if the standards defining your file format as only accepting ASCII line break characters.

    Since there are many such file formats, perhaps adding a parameter asciionly=True/False would make sense. .splitlines() could then be made to only split on ASCII linebreak characters. This new parameter would then have to default to False to maintain compatibility with Unicode and all previous releases.

    @nascheme
    Copy link
    Member

    nascheme commented Oct 5, 2018

    I've created a topic on this inside the "Ideas" area of discuss.python.org. Sorry if that wasn't appropriate, not sure if I should have keep the discussion here.

    Inada Naoki suggests creating a new method str.iterlines{[keepends]). Given that people are -1 on changing str.splitlines, I think that's a good solution. A new method is better yet if it would only split on '\n', that way fp.read().iterlines() matches fp.readlines(). It is what people seem to expect and is the most handy behaviour. So, str and bytes would both get the new method and they would both split on only '\n'.

    If we do that, I think nearly every use of splitlines() should get changed to iterlines().

    @malemburg
    Copy link
    Member

    Why not simply add a new parameter, to make people who want
    ASCII linebreaks continue to use .splitlines() ?

    It think it would be less than ideal to have one method break on
    all Unicode line breaks and another only on ASCII ones.

    @nascheme
    Copy link
    Member

    nascheme commented Oct 5, 2018

    Why not simply add a new parameter, to make people who want ASCII linebreaks continue to use .splitlines() ?

    That could work but I think in nearly every case you don't want to use splitlines() without supplying the parameter. So, it seems like a bit of trap for new users. Worse, because in Python 2, str.splitlines() does what they want, they will do the simple thing which is likely wrong.

    If we do stick with just splitlines(), perhaps it should get a 'newline' parameter that mostly matches io.open (i.e. it controls universal newline behavior). So if you don't want to change behavior, str.splitlines(newline=None) would split as it currently does. To make it split like io files do, you would have to do newline='\n'.

    To me, it seems attractive that:
    fp.readlines() == fp.read().iterlines()

    You suggestion would make it something like:
    fp.readlines() == fp.read().splitlines(newline='\n')

    I guess I could live with that but it seems unnecessarily ugly and verbose for what is the most common usage.

    @stevendaprano
    Copy link
    Member

    I don't like the idea of adding a second bool parameter to splitlines. Guido has a rough rule of thumb (which I agree with) of "no constant bool parameters". If people will typically call a function with some sort of "mode" parameter using a hard-coded bool, then we should usually prefer to split the two modes into distinct functions.

    As an example, we have statistics.stdev and pstdev rather than stdev(data, population=False).

    Obviously this is a guideline, not a hard rule, and there are exceptions. Such as str.splitlines :-)

    In any case, I suggest a separate string method. Even though the name is slightly inaccurate, I suggest "ascii_splitlines" which I think is accurate enough to capture the spirit of what we intend (split on *only* \n \r and \r\n) and we can leave the details in the docs.

    @nascheme
    Copy link
    Member

    nascheme commented Oct 7, 2018

    I too would prefer a new method name rather than overloading splitlines() with more keyword args (passed as hardcoded constants, usually). Again, I think we want:

    list(open(..).read().<splitmethod>()) == list(open(..))

    readlines() returns a list but I think this method should return an iterator (seems more Python 3 like to me, call list if you want a list). In that case, iterlines() seems like the right name to me. I think it should take a 'newline' keyword that behaves the same as the open() version of the keyword.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    9 participants