[Erp5-dev] [Erp5-report] r26242 - in /erp5/trunk/products/ERP5: Tool/ tests/

Discussion:

Yoshinori Okuji

2009-04-02 03:51:34 UTC

I don't understand why you need to *deprecate* cmp.

YO

Author: jm
Date: Wed Apr 1 17:01:12 2009
New Revision: 26242
URL: http://svn.erp5.org?rev=26242&view=rev
Deprecate sort_method parameter in DomainTool.searchPredicateList since cmp
parameter in sorting methods is slow and deprecated. Add new
sort_key_method parameter that is passed as key parameter to list.sort.
erp5/trunk/products/ERP5/Tool/DomainTool.py
erp5/trunk/products/ERP5/tests/testDomainTool.py
Modified: erp5/trunk/products/ERP5/Tool/DomainTool.py
http://svn.erp5.org/erp5/trunk/products/ERP5/Tool/DomainTool.py?rev=26242&r
1=26241&r2=26242&view=diff
===========================================================================
=== --- erp5/trunk/products/ERP5/Tool/DomainTool.py [utf8] (original)
+++ erp5/trunk/products/ERP5/Tool/DomainTool.py [utf8] Wed Apr 1 17:01:12
ignored_category_list=None,
tested_base_category_list=None,
filter_method=None, acquired=1,
# XXX: about "strict" parameter: This is a transition parameter,
# allowing someone hitting a bug to revert to original behaviour
easily. # It is not a correct name, as pointed out by Jerome. But instead
"""
Search all predicates which corresponds to this particular
context.
-
- - The sort_method parameter allows to give a method which will be
- used in order to sort the list of predicates found. The most
+
+ - sort_method parameter is deprecated: use sort_key_method instead.
+
+ - sort_key_method parameter is passed to list.sort as key parameter
if it + is not None. This allows to sort the list of predicates
found. The most important predicate is the first one in the list.
- ignored_category_list: this is the list of category that we do
@@ -203,8 +205,12 @@
# LOG('searchPredicateList, result_list before sort', 0,
result_list = filter_method(result_list)
- result_list.sort(sort_method)
+ result_list.sort(key=sort_key_method)
+ LOG('searchPredicateList', 0,
+ 'sort_method parameter is deprecated: sort_key_method
instead') + result_list.sort(cmp=sort_method)
# LOG('searchPredicateList, result_list after sort', 0, result_list)
return result_list
Modified: erp5/trunk/products/ERP5/tests/testDomainTool.py
http://svn.erp5.org/erp5/trunk/products/ERP5/tests/testDomainTool.py?rev=26
242&r1=26241&r2=26242&view=diff
===========================================================================
=== --- erp5/trunk/products/ERP5/tests/testDomainTool.py [utf8] (original)
+++ erp5/trunk/products/ERP5/tests/testDomainTool.py [utf8] Wed Apr 1
return -1
return 0
+ hasCellContent = getattr(x, 'hasCellContent', None)
+ return bool(hasCellContent and hasCellContent(base_id='path'))
+
get_transaction().commit()
self.tic()
domain_tool = self.getDomainTool()
@@ -384,16 +388,24 @@
'variation/%s/blue' %
self.resource.getRelativeUrl()]) mapped_value =
domain_tool.generateMappedValue(context,sort_method=sort_method)
self.assertEquals(mapped_value.getProperty('base_price'),45)
+ mapped_value =
domain_tool.generateMappedValue(context,sort_key_method=sort_key_method) +
self.assertEquals(mapped_value.getProperty('base_price'),45)
context = self.resource.asContext(
categories=['resource/%s' %
self.resource.getRelativeUrl(), 'variation/%s/red' %
self.resource.getRelativeUrl()]) mapped_value =
domain_tool.generateMappedValue(context,sort_method=sort_method) +
self.assertEquals(mapped_value.getProperty('base_price'),26)
+ mapped_value =
domain_tool.generateMappedValue(context,sort_key_method=sort_key_method)
self.assertEquals(mapped_value.getProperty('base_price'),26)
# Now check the price
self.assertEquals(self.resource.getPrice(context=self.resource.asContext(
categories=['resource/%s' % self.resource.getRelativeUrl(),
'variation/%s/blue' % self.resource.getRelativeUrl()]),
sort_method=sort_method),45)
+
self.assertEquals(self.resource.getPrice(context=self.resource.asContext( +
categories=['resource/%s' %
self.resource.getRelativeUrl(), + 'variation/%s/blue' %
self.resource.getRelativeUrl()]), +
sort_key_method=sort_key_method),45)
def test_06_SQLQueryDoesNotReturnTooManyPredicates(self, quiet=0,
run=run_all_test): if not run: return
_______________________________________________
Erp5-report mailing list
Erp5-report at erp5.org
http://mail.nexedi.com/mailman/listinfo/erp5-report

--
Yoshinori Okuji, Nexedi KK President
Nexedi: Consulting and Development of Free / Open Source Software
http://www.nexedi.co.jp/
ERP5: Full Featured High End Open Source ERP
http://www.erp5.com/
ERP5 Wiki: Developer Zone for ERP5 Community
http://www.erp5.org/

Julien Muchembled

2009-04-02 14:34:33 UTC

Permalink

Post by Yoshinori Okuji
I don't understand why you need to *deprecate* cmp.

I explained in the commit message that using 'cmp' parameter in sorting methods (list.sort & sorted) is slow and deprecated. This is a fact.

More precisely:

* In almost all cases, 'cmp' is _much_ slower that 'key'. In the rare cases where using 'cmp' could be faster, it is possible to write a 'key' func that behaves like a 'cmp': cf
http://code.activestate.com/recipes/576653/ (Convert a cmp function to a key function).

* 'cmp' parameter doesn't exist in Python 3 (and I suppose it will also disappear in a future 2.x version). I know this is really far future but it appears we already need to convert existing code due to performance issues.

Knowing all this, it is not serious to keep using 'cmp' in new code.

Julien

Post by Yoshinori Okuji

Yoshinori Okuji

2009-04-02 15:00:40 UTC

Permalink

Post by Julien Muchembled

Post by Yoshinori Okuji
I don't understand why you need to *deprecate* cmp.

I explained in the commit message that using 'cmp' parameter in sorting
methods (list.sort & sorted) is slow and deprecated. This is a fact.

Slowness does not mean any deprecation.

Post by Julien Muchembled
* In almost all cases, 'cmp' is _much_ slower that 'key'. In the rare cases
where using 'cmp' could be faster, it is possible to write a 'key' func
that behaves like a 'cmp': cf http://code.activestate.com/recipes/576653/
(Convert a cmp function to a key function).

I don't care about whether it is fast or slow. The fact is that it is quite
difficult to write a key function in some cases. Note that the complexity of
the workaround given there is equal to using cmp. So it is an "aim" for
dealing with Python 3's bad decision, but not for the performance.

Post by Julien Muchembled
* 'cmp' parameter doesn't exist in Python 3 (and I suppose it will also
disappear in a future 2.x version). I know this is really far future but it
appears we already need to convert existing code due to performance issues.
Knowing all this, it is not serious to keep using 'cmp' in new code.

Not at all. You can recommend using key whenever feasible, but you should not
force to use key only.

Regards,
YO

Julien Muchembled

2009-04-02 15:48:26 UTC

Permalink

Post by Yoshinori Okuji

I don't care about whether it is fast or slow. The fact is that it is quite
difficult to write a key function in some cases.

In almost all cases, key functions are smaller, and easier to write. To test according several properties (one after the other, until a difference is found), you can return a list.
The recipe could be used when the key form is slower or hard to maintain.

Post by Yoshinori Okuji
Note that the complexity of
the workaround given there is equal to using cmp.

I know.

Post by Yoshinori Okuji

Not at all. You can recommend using key whenever feasible, but you should not
force to use key only.

Enforcing things avoid the need to document and avoid the case where someone didn't read that document.

Here, the performance hit is so high that we shouldn't allow people using a cmp function by mistake.

Also, I don't like the idea of adding countless feature-parameters to API method just because that parameter might useful one day. sort_key_method is enough for DomainTool.searchPredicateList.

BTW, this is my last mail on this subject because I have no time to discuss further. Do what you want.

Julien

Yoshinori Okuji

2009-04-02 17:09:45 UTC

Permalink

Post by Julien Muchembled
In almost all cases, key functions are smaller, and easier to write. To
test according several properties (one after the other, until a difference
is found), you can return a list. The recipe could be used when the key
form is slower or hard to maintain.

You may not write a class in restricted environment.

Post by Julien Muchembled
BTW, this is my last mail on this subject because I have no time to discuss
further. Do what you want.

I have removed the deprecated warning.

Regards,
YO