Re: [Freeipa-devel] [PATCH 0066] ipactl: Do not start/stop/restart single service multiple times

2015-10-13 Thread Tomas Babej


On 08/27/2015 08:07 AM, David Kupka wrote:
> On 26/08/15 17:49, Tomas Babej wrote:
>>
>>
>> On 08/26/2015 03:16 PM, David Kupka wrote:
>>> https://fedorahosted.org/freeipa/ticket/5248
>>>
>>>
>>
>> +def deduplicate(lst):
>> +new_lst = []
>> +s = set(lst)
>> +for i in lst:
>> +if i in s:
>> +s.remove(i)
>> +new_lst.append(i)
>> +
>> +return new_lst
>> +
>>
>> Imho, this method deserves a docstring or at least a comment. It is not
>> entrirely clear from the name, that its job is to remove the duplicates
>> while preserving the order of the entries.
>>
> 
> You're right, line or two could not hurt. Patch attached.

Obvious ACK from me.

Pushed to:
master: 5ff4170ff9cab64d3527001de8214cb30439e3e3
ipa-4-2: 9e3d0d6f591496a2370a14832f7c0c399a16b2ea

-- 
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


Re: [Freeipa-devel] [PATCH 0066] ipactl: Do not start/stop/restart single service multiple times

2015-08-26 Thread David Kupka

On 26/08/15 17:49, Tomas Babej wrote:



On 08/26/2015 03:16 PM, David Kupka wrote:

https://fedorahosted.org/freeipa/ticket/5248




+def deduplicate(lst):
+new_lst = []
+s = set(lst)
+for i in lst:
+if i in s:
+s.remove(i)
+new_lst.append(i)
+
+return new_lst
+

Imho, this method deserves a docstring or at least a comment. It is not
entrirely clear from the name, that its job is to remove the duplicates
while preserving the order of the entries.



You're right, line or two could not hurt. Patch attached.


Anyway, deduplication can be implemented in a more readable way:


from collections import OrderedDict
sample_list = [3,2,1,2,1,5,3]
OrderedDict.fromkeys(sample_list).keys()

[3, 2, 1, 5]


I know about this approach and it does not seem much more readable to 
me. I just chosen few more lines instead of an import.




Tomas



--
David Kupka
From 8f8a97747968ae9b69b1f7d0b9deaab730feba4c Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 27 Aug 2015 07:34:02 +0200
Subject: [PATCH] comment: Add Documentation string to deduplicate function

---
 install/tools/ipactl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/install/tools/ipactl b/install/tools/ipactl
index 5782d4c424fb98c08e55614c71f3abaa6e776a68..379a8d91b2419a9708919ac9713352fcc242e79f 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -46,6 +46,9 @@ def check_IPA_configuration():
   "(see man pages of ipa-server-install for help)", 6)
 
 def deduplicate(lst):
+"""Remove duplicates and preserve order.
+Returns copy of list with preserved order and removed duplicates.
+"""
 new_lst = []
 s = set(lst)
 for i in lst:
-- 
2.4.3

-- 
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

Re: [Freeipa-devel] [PATCH 0066] ipactl: Do not start/stop/restart single service multiple times

2015-08-26 Thread Tomas Babej


On 08/26/2015 03:16 PM, David Kupka wrote:
> https://fedorahosted.org/freeipa/ticket/5248
> 
> 

+def deduplicate(lst):
+new_lst = []
+s = set(lst)
+for i in lst:
+if i in s:
+s.remove(i)
+new_lst.append(i)
+
+return new_lst
+

Imho, this method deserves a docstring or at least a comment. It is not
entrirely clear from the name, that its job is to remove the duplicates
while preserving the order of the entries.

Anyway, deduplication can be implemented in a more readable way:

>>> from collections import OrderedDict
>>> sample_list = [3,2,1,2,1,5,3]
>>> OrderedDict.fromkeys(sample_list).keys()
[3, 2, 1, 5]

Tomas

-- 
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


Re: [Freeipa-devel] [PATCH 0066] ipactl: Do not start/stop/restart single service multiple times

2015-08-26 Thread Petr Vobornik

On 08/26/2015 03:16 PM, David Kupka wrote:

https://fedorahosted.org/freeipa/ticket/5248




ACK

Pushed to:
master: 59cc54b6dce29e32e81bfaad25ff13794092d782
ipa-4-2: 21cdcbd9a6b6a82d39d40b91a64d4d9b4d7e4e7d
--
Petr Vobornik

--
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