Handle bytes comparisons in difflib.Differ

[warsaw]

BPO	17445
Nosy	@warsaw, @terryjreedy, @ncoghlan, @pitrou, @bitdancer, @berkerpeksag
Files	bytes_diff.py: Wrap difflib.context/unified_diff 13161c1d9c5f.diff 13161c1d9c5-difflibf.diff fa4c6160c518.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 = <Date 2015-04-21.01:22:35.351>
created_at = <Date 2013-03-17.14:42:58.733>
labels = ['type-bug', 'library']
title = 'Handle bytes comparisons in difflib.Differ'
updated_at = <Date 2015-04-21.03:06:42.469>
user = 'https://github.com/warsaw'

bugs.python.org fields:

activity = <Date 2015-04-21.03:06:42.469>
actor = 'berker.peksag'
assignee = 'gward'
closed = True
closed_date = <Date 2015-04-21.01:22:35.351>
closer = 'gward'
components = ['Library (Lib)']
creation = <Date 2013-03-17.14:42:58.733>
creator = 'barry'
dependencies = []
files = ['29493', '39053', '39057', '39083']
hgrepos = ['179']
issue_num = 17445
keywords = ['patch']
message_count = 29.0
messages = ['184379', '184381', '184382', '184409', '184424', '184428', '184438', '184487', '184490', '184534', '184535', '184547', '184566', '184580', '184596', '184597', '184699', '185157', '240717', '240718', '241098', '241107', '241172', '241174', '241175', '241239', '241302', '241304', '241691']
nosy_count = 9.0
nosy_names = ['barry', 'gward', 'terry.reedy', 'ncoghlan', 'pitrou', 'durin42', 'r.david.murray', 'python-dev', 'berker.peksag']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17445'
versions = ['Python 3.5']

[warsaw]

This came up at the Pycon 2013 Python 3 porting clinic. There are many cases in the stdlib that claim (either explicitly or implicitly) to accept bytes or strings, but that don't return the type of the arguments they accept. An example is urllib.parse.quote() which accepts bytes or str but always returns str. A similar example brought up at the clinic was difflib, which accepts both types, and works internally on both, but crashes when joining the results for return.

It should be policy for the stdlib (i.e. codified in an informational PEP and including bug reports, because they *are* bugs, not features or baked-in API) where bytes or str are accepted but the right things are not done (i.e. return the type you accept).

This bug captures the principle, and probably should be closed once such a PEP is accepted, with individual bugs opened for each individual case.

[bitdancer]

There was a long thread about this on python-dev that might be worth going back over, where I had the same misconception (that functions should always return the same type as their arguments). While I think that should be the default design, it isn't always the best API. (The real rule, if I recall correctly, is that functions should never accept *mixed* argument types for input data.)

[warsaw]

On Mar 17, 2013, at 03:10 PM, R. David Murray wrote:

There was a long thread about this on python-dev that might be worth going
back over, where I had the same misconception (that functions should always
return the same type as their arguments). While I think that should be the
default design, it isn't always the best API. (The real rule, if I recall
correctly, is that functions should never accept *mixed* argument types for
input data.)

Totally agree about the mixed type rule.

But this is something different, and I know it's been discussed, and is a
difficult problem. It's causing real-world pain for people though, so it's
worth thinking about again.

[gward]

The particular use case that triggered this: Mercurial's test suite. It runs "hg blah blah" and compares the output against known good output. But Mercurial's output is just bytes, because pretty much everything in a Mercurial repo is just bytes (file data of course, but also filenames and even changeset metadata like usernames).

So attempting to run the Mercurial test suite under 3.x immediately fails hard. The boiled-down essence of the bug is this:

>>> import difflib
>>> a = b"hello world"
>>> b = b"goodbye world"
>>> [line for line in difflib.unified_diff(a, b)]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in <listcomp>
  File "/home/greg/src/cpython/3.2/Lib/difflib.py", line 1224, in unified_diff
    yield '-' + line
TypeError: Can't convert 'int' object to str implicitly

[bitdancer]

That looks like a bug in difflib (regardless of what type it returns). But note that I'm agreeing that returning the same type you are given is generally preferrable.

[terryjreedy]

Changing behavior that already matches the docs is an enhancement, not a bugfix, and one that will almost certainly break code. It is therefore one that would normally require a deprecation period. I think the most you should ask for is to skip the deprecation period.

I believe the urllib and difflib problems are quite different. I am going to presume that urllib simply converts bytes input to str and goes on from there, returning the result as str rather than (possibly) converting back to bytes. That is an example for this issue.

Difflib.unified_diff, on the other hand, raises rather than returning an unexpected or undesired type. The 3 sections like the below have two problems given the toy input of two bytes objects.

            if tag in {'replace', 'delete'}:
                for line in a[i1:i2]:
                    yield '-' + line

First, iterating bytes or a slice of bytes returns ints, not 1-byte bytes. Hence the exception. Even if that were worked around, the mixed string constant + bytes expression would raise a TypeError. One fix for both problems would be to change the expression to '-' + str(line).

Neither of these problems are bugs. The doc says "Compare a and b (lists of strings)". Actually, 'sequence of strings' is sufficient. For the operations of unified_diff, a string looks like a sequence of 1-char strings, which is why

>> for l in difflib.unified_diff('ab', 'c'): print(l)

---

+++

@@ -1,2 +1 @@

-a
-b
+c

works.

The other lines yielded by unified_diff are produced with str.format, and % formatting does not seem to work with bytes either. So a dual string/bytes function would not be completely trivial.

Greg, can you convert bytes to strings, or strings to bytes, for your tests, or do you have non-ascii codes in your bytes? Otherwise, I think it might be better to write a new function 'unified_diff_bytes' that did exactly what you want than to try to make unified_diff accept sequences of bytes.

[ncoghlan]

At a glance, this just looks like a bug in difflib - it should use
different literals when handling bytes. (Given that difflib newline
processing assumes ASCII compatibility, a latin-1 based decode/encode
solution may also be viable).

[gward]

The original reproduction I posted was incorrect -- it makes difflib look worse than it should. (I passed strings rather than lists of strings.) Here is a more accurate version:

>>> import difflib
>>> a = [b'hello']
>>> b = [b'hello!']
>>> '\n'.join(line for line in difflib.unified_diff(a, b))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in <genexpr>
  File "/home/greg/src/cpython/3.3/Lib/difflib.py", line 1223, in unified_diff
    yield '-' + line
TypeError: Can't convert 'bytes' object to str implicitly

So it still crashes, but the exception makes it pretty clear what the problem is.

[gward]

Replying to Terry Reedy:

So a dual string/bytes function would not be completely trivial.

Correct. I have one working, but it makes my eyes bleed. I fail ashamed to have written it.

Greg, can you convert bytes to strings, or strings to bytes

Nope. Here is the hypothetical use case: I have a text file written in Polish encoded in ISO-8859-1 committed to a Mercurial repository. (Or saved in a filesystem somewhere: doesn't really matter, except that Mercurial repositories are immutable, long-term, and *must* *not* *lose* *data*.) Then I decide I should play nicely with the rest of the world and transcode to UTF-8, so commit a new rev in UTF-8.

Years later, I need to look at the diff between those two old revisions. Rev 1 is a pile of ISO-8859-2 bytes, and rev 2 is a pile of UTF-8 bytes. The output of diff looks like

• blah blah [iso-8859-2 bytes] blah
  + blah blah [utf-8 bytes] blah

Note this: the output of diff has some lines that are iso-8859-2 bytes and some that are utf-8 bytes. *There is no single encoding* that applies.

Note also that diff output must contain the exact original bytes, so that it can be consumed by patch. Diffs are read both by humans and by machines.

Otherwise, I think it might be better to write a new function
'unified_diff_bytes' that did exactly what you want than to try to
make unified_diff accept sequences of bytes.

Good idea. That might be much less revolting than what I have now. I'll give it a shot.

[gward]

OK I now have two competing patches. Both are disgusting, but in different ways.

1. http://hg.gerg.ca/cpython/rev/fcf3d27f20d9

   • factor out all the string constants
   • always concatenate, do not .format()

2. http://hg.gerg.ca/cpython/rev/cebefce2cfd4

   • copy {unified,context}_diff() to {unified,context}_diff_bytes()
   • this is a future maintenance headache, guaranteed!

Feedback welcome. If anyone can see a way to unify these two approaches, or a third way that sucks less, I'm all ears.

[pitrou]

Necessary when comparing files with unknown or inconsistent encodings:
there's no way to convert everything to Unicode, so just leave it all
bytes.

You could simply use the surrogateescape error handler.

[ncoghlan]

Since we don't need to worry about ASCII incompatible encodings (difflib will already have issues with such files due to the assumptions about newlines), it should be possible to use the same approach as that used in urllib.parse, but based on latin-1 rather than ascii.

It's the least bad option for this kind of use case (surrogateescape can be good too, but it doesn't work properly in this case where the two encodings may be different and we want to compare the raw bytes directly).

(changed scope of issue to reflect the subsequent discussion)

[gward]

Take 3: http://hg.gerg.ca/cpython/rev/78bdb10551ee

• uses surrogateescape as suggested by Antoine
• seems to work