Bug#625509: (no subject)

Colin Watson cjwatson at ubuntu.com
Wed Jun 13 17:26:28 UTC 2012 

On Thu, Apr 26, 2012 at 05:39:03PM -0400, Barry Warsaw wrote:
> I'm starting to review and test Colin's patches.  Here are some comments.
> Where I don't mention anything (the majority of the code), then I think it
> looks great.

Likewise, where I don't mention anything, it's because I've addressed
your concerns more or less as stated.

> Meta-question: do you think it makes sense to turn on `from __future__ import
> unicode_literals`?

We've talked about this on a few occasions. :-)

This is more or less the poster child for a case where I think
unicode_literals is difficult.  There's a complex API which is quite
sensitive to whether things are bytes or unicode in places, and often
handles both.  The tests need to check whether both are handled.  It
interacts with several other interfaces that prefer to work with bytes
in Python 2 but Unicode strings in Python 3.

In short, I did try, but I found it to be much more complicated to try
to shift to unicode_literals than it was worth, and I really wasn't
confident enough in absolute 100% test coverage to be certain I hadn't
broken anything.

> 0003-Use-Python-3-style-print
> 
> diff --git a/lib/debian/arfile.py b/lib/debian/arfile.py
> diff --git a/lib/debian/debfile.py b/lib/debian/debfile.py
> diff --git a/lib/debian/debian_support.py b/lib/debian/debian_support.py
> diff --git a/lib/debian/debtags.py b/lib/debian/debtags.py
> diff --git a/lib/debian/doc-debtags b/lib/debian/doc-debtags
> 
> You need to add `from __future__ import print_function` to these files.

Technically not in all of them since a single-element tuple prints the
same way as its element. :-)  But that wasn't in the case in all of
them, and that assumption is fragile in any case, so I've made this
change.

> 0005-Use-iterkeys
> 
> Why make these changes, given that .iterkeys() and .iteritems() doesn't exist
> in Python 3?  I see later you use `six` for Python version compatibility, but
> why not just use .keys() and .items() instead?  Are these collections so big
> that making concrete lists in Python 2 will cause serious memory pressure?

Mm, I think at the time I was trying to make things work nicely with
2to3 or something.  You're right that it's unlikely to matter.  I've
replaced this with a commit that replaces all "for key in dict.keys()"
and "for key in dict.iterkeys()" with "for key in dict", per your above
comment, thereby reducing the need for six later on.

> 0011-Implement-rich
> 
> This looks fine.  If you wanted to, and you can support nothing less that
> Python 2.7 (not yet true for Debian), then you could define just __lt__ and
> __eq__ and let the @functools.total_ordering class decorator fill in the
> rest.  Just FYI.

I've added TODO comments for this.

> 0022-Be-much-more-careful
[...]
> It's a test, so this doesn't bother me too much, and also because while you
> could use contextlib.nested() in Python 2, that doesn't exist in Python 3.
> There, and in Python 2.7, the with-statement itself supports multiple context
> managers, so you could write:
> 
>     with open(filename, 'rb') as f1, open(filename, 'rb') as f2:
> 
> Unfortunately, that doesn't work in Python 2.6. :(

Indeed.  I looked at this at the time and decided it wasn't worth the
hassle for now; but I've added a TODO comment.

Here's an updated patch series, with a number of other minor changes
inspired by your commentary and my own experience of porting other
things to Python 3 since then.  I'd love a maintainer comment on this,
or if all else fails an indication that I should just NMU the lot. :-)
I'd really rather have maintainer review given the nature of the changes
though.

Thanks,

-- 
Colin Watson                                       [cjwatson at ubuntu.com]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-test-warnings-with-python2.7-3.patch
Type: text/x-diff
Size: 6395 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0031.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Avoid-various-old-syntactic-forms-which-are-no-longe.patch
Type: text/x-diff
Size: 10291 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0032.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Use-Python-3-style-print-function.patch
Type: text/x-diff
Size: 8740 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0033.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Use-a-list-comprehension-instead-of-map-which-return.patch
Type: text/x-diff
Size: 800 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0034.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Rely-on-default-mapping-iteration-rather-than-using-.patch
Type: text/x-diff
Size: 2421 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0035.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Use-absolute-imports.patch
Type: text/x-diff
Size: 6651 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0036.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-Use-Python-3-style-print-function-in-examples.patch
Type: text/x-diff
Size: 25596 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0037.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-Use-key-in-dict-rather-than-obsolete-dict.has_key-ke.patch
Type: text/x-diff
Size: 2158 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0038.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-Use-open-rather-than-file-file-does-not-exist-in-Pyt.patch
Type: text/x-diff
Size: 11497 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0039.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-Use-sep.join-list-rather-than-string.join-list-sep.patch
Type: text/x-diff
Size: 2697 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0040.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0011-Implement-rich-comparison-methods-the-only-kind-avai.patch
Type: text/x-diff
Size: 5354 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0041.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0012-Use-assertTrue-and-assertEqual-rather-than-deprecate.patch
Type: text/x-diff
Size: 6321 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0042.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0013-Try-to-import-pickle-if-importing-cPickle-fails.-Pyt.patch
Type: text/x-diff
Size: 1253 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0043.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0014-Use-io.StringIO-if-StringIO.StringIO-is-absent-as-in.patch
Type: text/x-diff
Size: 2166 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0044.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0015-Use-collections.Mapping-collections.MutableMapping-i.patch
Type: text/x-diff
Size: 3924 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0045.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0016-Use-list-comprehensions-instead-of-map-where-a-list-.patch
Type: text/x-diff
Size: 3143 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0046.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0017-If-StandardError-does-not-exist-as-in-Python-3-inher.patch
Type: text/x-diff
Size: 1579 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0047.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0018-Use-six-to-paper-over-dict-iteration-differences-bet.patch
Type: text/x-diff
Size: 6653 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0048.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0019-Avoid-using-0L-since-Python-3-doesn-t-support-it-0-w.patch
Type: text/x-diff
Size: 1237 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0049.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0020-Cope-with-the-absence-of-a-file-class-in-Python-3.patch
Type: text/x-diff
Size: 1498 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0050.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0021-Python-3-renamed-raw_input-to-input.patch
Type: text/x-diff
Size: 772 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0051.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0022-Be-much-more-careful-about-closing-files-in-a-timely.patch
Type: text/x-diff
Size: 9931 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0052.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0023-Use-next-iterator-rather-than-iterator.next.patch
Type: text/x-diff
Size: 4343 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0053.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0024-Use-string.ascii_letters-rather-than-the-deprecated-.patch
Type: text/x-diff
Size: 865 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0054.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0025-Encode-Unicode-strings-before-passing-them-to-hashli.patch
Type: text/x-diff
Size: 858 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0055.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0026-Fix-up-debian.changelog-for-string-handling-changes-.patch
Type: text/x-diff
Size: 6220 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0056.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0027-Only-define-DebPart.has_key-method-for-Python-2.patch
Type: text/x-diff
Size: 1044 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0057.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0028-Fix-up-debian.arfile-and-debian.debfile-for-string-h.patch
Type: text/x-diff
Size: 13593 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0058.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0029-Fix-up-most-of-debian.deb822-for-string-handling-cha.patch
Type: text/x-diff
Size: 9259 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0059.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0030-Fix-up-the-rest-of-debian.deb822-for-Python-3-string.patch
Type: text/x-diff
Size: 15491 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0060.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0031-Add-a-python3-debian-package.patch
Type: text/x-diff
Size: 3338 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20120613/1f0f6e8e/attachment-0061.patch>

More information about the pkg-python-debian-maint mailing list