[Freeipa-devel] [PATCHES] 0696-0710 More modernization

[Christian Heimes]

On 2015-08-21 12:55, Petr Viktorin wrote:
> On 08/14/2015 07:44 PM, Petr Viktorin wrote:
>> Hello,
>> These patches bring IPA another step towards compatibility with Python 3.
>>
>> Most of these were made by fixers from the "python-modernize" tool, but
>> I reviewed and edited the results.
> 
> Here are the patches rebased to current master.

0696.2-Remove-use-of-sys.exc_value
ACK


0697.2-Don-t-use-a-tuple-in-function-arguments
I prefer operator.itemgetter() over the hard-to-read lambda expression
key=lambda k_v: (k_v[1], k_v[0]).
>>> import operator
>>> example = dict(a=3, ba=2, b=2, c=1)
>>> sorted(example.items(), key=operator.itemgetter(1, 0))
[('c', 1), ('b', 2), ('ba', 2), ('a', 3)]


0698.2-Add-python-six-to-dependencies
ACK


0699.2-Remove-the-unused-pygettext-script
ACK


0700.2-Use-six.string_types-instead-of-basestring
LGTM, but I need to have a closer look at some places.
I noticed a couple of asserts that should be "if ... raise ValueError"
instead. python -o disables asserts.


0701.2-Use-Python3-compatible-dict-method-names
NACK
Why are you replacing iteritems() with items() instead of using
six.iteritems()?
Please use sorted(reference) instead of sorted(reference.keys()),
set(tree) instead of set(tree.keys()) and list(somedict) instead of
list(somedict.keys()), too. The keys() call is unnecessary and frowned upon.


0702.2-Replace-filter-calls-with-list-comprehensions
In Python 2 list comprehensions leak the internal loop variable. It
might be better to write a generator expression with list() instead of
[] list comprehension.


0703.2-Use-six.moves.input-instead-of-raw_input
ACK
The code is fine, but pylint won't like it. For Dogtag I had to disable
pylint warnings W0622 and F0401.


0704.2-Use-six.integer_types-instead-of-long-int
ACK
hint: For type checks you can also use the numbers module.


0705.2-Replace-uses-of-map
See comment for 0702


706.2-Use-next-function-on-iterators
ACK


0707.2-Use-the-print-function
LGTM
There are too many chances to review. Let's hope the automatic
conversion tool did its job correctly.


0708.2-Use-new-style-raise-syntax
ACK


0709.2-Use-six.reraise
ACK


0710.2-Modernize-use-of-range
NACK
Please use six.moves.range. It defaults to xrange() in Python 2. I also
see a couple of additional opportunities for enumerate():

for i in range(len(kw['attrs'])):
    kw['attrs'][i] = unicode(kw['attrs'][i])

for i, s in enumerate(kw['attrs']):
    kw['attrs'][i] = unicode(s)


0711.2-Convert-zip-result-to-list
ACK
The code isn't beautiful but it's just a test.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150821/05066fbe/attachment.sig>