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 <dku...@redhat.com>
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

Reply via email to