On 21.8.2015 14:50, Christian Heimes wrote:
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

These are generator objects in ipapython/install/core.py. I'm not sure what the usual convention is, but I would think that the gen.next() calls should be replaced with gen.send(None) instead of next(gen), so that the generators are accessed consistently using methods (gen.send()/gen.throw()/gen.close()).



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

Instead of calling six.reraise from raise_exc_info, could you replace the two occurences of raise_exc_info(exc_info) with six.reraise(*exc_info) and remove raise_exc_info?



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.






--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to