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:
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.
I prefer operator.itemgetter() over the hard-to-read lambda expression
key=lambda k_v: (k_v, k_v).
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)]
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.
Why are you replacing iteritems() with items() instead of using
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.
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.
The code is fine, but pylint won't like it. For Dogtag I had to disable
pylint warnings W0622 and F0401.
hint: For type checks you can also use the numbers module.
See comment for 0702
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
There are too many chances to review. Let's hope the automatic
conversion tool did its job correctly.
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?
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)
The code isn't beautiful but it's just a test.
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code