classification
Title: Get rid of more references to __cmp__
Type: behavior Stage: patch review
Components: Interpreter Core Versions: Python 3.0, Python 3.1
process
Status: closed Resolution: fixed
Dependencies: 4704 Superseder:
Assigned To: georg.brandl Nosy List: amaury.forgeotdarc, atuining, benjamin.peterson, brett.cannon, christian.heimes, georg.brandl, gpolo, lemburg, mark.dickinson, pitrou, python-dev, rhettinger
Priority: normal Keywords: patch

Created on 2008-01-01 16:39 by gvanrossum, last changed 2013-08-22 01:14 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
nocmp.diff gvanrossum, 2008-01-01 16:39
nocmp_tests.diff mark.dickinson, 2008-10-16 07:20 Fixes for failing tests
remove_cmp_decimal.patch mark.dickinson, 2008-12-07 17:31 Fix cmp in decimal module and tests
remove_cmp2.patch christian.heimes, 2008-12-07 18:15
remove_cmp3.patch mark.dickinson, 2008-12-07 21:03
remove_cmp4.patch christian.heimes, 2008-12-08 01:48
remove_cmp5.patch mark.dickinson, 2008-12-08 11:23
remove_cmp6.patch mark.dickinson, 2009-01-02 15:09
1717_stage1.patch mark.dickinson, 2009-01-25 12:14
1717_stage1_v2.patch mark.dickinson, 2009-01-25 13:00
1717_stage2.patch mark.dickinson, 2009-01-30 17:38
Messages (90)
msg59072 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-01-01 16:39
Should I apply this?  There are more places that reference __cmp__ in
the library.  OTOH there are some folks who would like to see __cmp__
make a come-back as a shorthand for defining 6 comparison operators, for
totally-ordered types.  (I'm still waiting for a PEP describing this
though.)  Even in that case I'm not sure that the code I'm proposing to
delete here would be useful.
msg63894 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-03-18 05:07
__cmp__ is not coming back.
msg64668 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-03-29 01:30
Bumping priority.
msg70472 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-31 02:06
Ping
msg70745 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-08-05 16:00
Can someone other than me test and apply this?  It seems still relevant,
and __cmp__ is not coming back.
msg70846 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-08-07 18:15
Additionally, there are still lots of references to __cmp__ in the
library which should be ripped out.
msg72903 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-09-09 20:12
Bumping priority even further. This shouldn't make it past rc.
msg72924 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-09 22:01
Guido's patch breaks these tests:

test_descr test_hash test_long test_richcmp test_set
msg72939 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-09-10 02:41
Since we are making 3.0 issues deferred blockers dropping the priority.
msg74829 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-10-16 07:20
> Guido's patch breaks these tests:
> 
> test_descr test_hash test_long test_richcmp test_set

It looks like all these are easily fixed:  all these tests were making 
outdated assumptions and needed updating.

Here's a patch that fixes these tests.

One other detail:

In test_descr.py, there are tests for 'overridden behavior for static 
classes' and 'overridden behavior for dynamic classes', using test classes 
'Proxy' and 'DProxy';  apart from the name change, the 'dynamic' code is 
identical to the 'static' code, so I removed it.  I guess this had to do 
with the __dynamic__ class attribute, which is ancient history, no?
msg74848 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-10-16 19:35
Thanks, Mark! Applied in r66920.
msg74849 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-10-16 19:37
The library still has __cmp__ functions defined here and there, e.g. in
xmlrpc/client.py.
msg74854 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-10-16 20:33
Presumably any nonzero entries for tp_compare in type initializers 
should be looked at closely, as well?

I see nonzero tp_compare entries in:
  Modules/_tkinter.c
  Modules/parsermodule.c
  Objects/cellobject.c
  Objects/descrobject.c
  PC/winreg.c
  Objects/setobject.c
(but that last one just raises an error pointing out that cmp can't be 
used to compare sets, so maybe that's okay).
msg74857 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-10-16 20:55
Let's lower the priority on this.
msg77105 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-12-06 08:15
cmp() the function is also still there.
msg77143 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-12-06 16:42
Bah. That means we'll have to start deprecating cmp() in 3.1, and won't
be able to remove it until 3.2 or 3.3. :-)
msg77173 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-12-06 22:09
I hope the smiley really indicates a joke...
msg77174 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-12-06 22:12
Well what would you suggest we do?
msg77175 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-12-06 22:15
I'd release 3.0.1 quickly, maybe also with the in-development
improvements to the io library that alleviate the factor-1000 slowdowns.
msg77182 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-06 22:58
Since cmp() was documented as removed in whatsnew3.0, it may be fair
game to complete the removal, treating it as a bugfix.  

If as Georg suggests it is done *very* quickly in 3.0.1 (say in the next
week or so), it is likely harmless and better than living with a
half-removed feature for the next three years.  Also, Georg intimated,
we will likely need to do 3.0.1 almost immediately anyway (the current
state of affairs with IO borders on the unusable.
msg77184 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-06 23:04
A quick ballot on #python-dev resulted in 3 of 3 votes for removing
cmp() in a quick bug fix release.

About the io misery: 
My quick fix for the buffer reallocation schema has made the situation a
bit better. For the 3.0.x series I like to discuss if we shouldo declare
critical performance fixes as bug fixes and not as new features.
msg77187 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-12-06 23:24
OK, remove it in 3.0.1, provided that's released this year.

Performance fixes are always fair game for bugfix releases.

Please don't "fix" the what's new document (or undo the fix).

I do hope cmp() was already undocumented elsewhere.
msg77189 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-12-06 23:39
Do the API functions PyObject_Compare() and PyObject_Cmp() also go away?
msg77191 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-12-06 23:45
Do we also get rid of the tp_compare type slot? 6 types still use it, 
they should convert this to a tp_richcompare slot
msg77192 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-12-06 23:47
Yes, tp_compare should go.
msg77199 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-12-07 00:23
tp_richcompare is less convenient to implement than tp_compare. See for 
example how in Objects/longobject.c, long_richcompare() calls long_compare() *and* a helper method Py_CmpToRich.

Likewise, turning __cmp__ into the whole set of __lt__, __le__, __ge__, 
__gt__, __eq__, __ne__ methods (did I forget one?) is tedious.

I'm not talking for my own code, but for the python standard objects and 
library (and the Demo directory) where I'm currently trying to remove 
__cmp__ and tp_compare.

I am not far to write a hack (mixin, metaclass) to inject rich 
comparison functions based on a single __cmp__ function.
msg77203 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-07 01:14
Either take out the C-API functions or re-define them as:

   lambda a, b: (a > b) - (a < b)
msg77206 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-07 01:50
r67627 : complete the dedocumenting of cmp().

Amaury, I have a non-metaclass solution for you.  Will post on ASPN.  It
uses a class decorator to sniff-out *any* defined rich ordering
comparisons and the automatically fills-in those that are missing.
msg77207 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-07 01:58
FWIW, the tp_compare slot in setobject.c serves only say the cmp() isn't
defined.  It can be ripped-out straight-away.
msg77220 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-07 12:19
+1 for a speedy removal of cmp and tp_compare
msg77222 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-07 12:36
We shouldn't remove the tp_compae slot in 3.0. That's going to break too
much 3rd party software. Instead of removing it, a deprecation warning
should be printed when the slot isn't 0.
msg77231 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-07 16:11
Here is a longer patch that removes cmp, PyObject_Compare and cmpfunc.
The slots has been renamed to tp_reserved. If the slot is used by a type
an exception is raised to notify the user about possible issues.

Missing:
* fix unit tests
* add error checks to PyObject_RichCompareBool calls
* Remove/replace the last PyUnicode Compare ASII function
msg77235 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-07 17:26
I'll work on fixing the unit tests if that's helpful.
msg77236 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-07 17:29
Can you fix the decimal module and tests? You know more about the module
than me. I'm half through most of the others modules.
msg77237 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-07 17:31
Decimal fixes
msg77239 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-07 18:15
I've integrated Mark's patch and fixed more tests. Who wants to pick it
up from here?
msg77255 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-07 21:03
remove_cmp3.patch adds to Christian's patch to fix 6 more test_failures:
(test_distutils, test_kqueue, test_list, test_sort, test_sqlite, 
test_userlist).

On OS X, the only remaining test failure is test_unittest.py.
msg77258 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-07 21:26
About unittest:

unittest.TestLoader has an attribute "sortTestMethodsUsing", which it 
expects to be an old-style comparison.

Should this attribute be updated (and renamed?) to expect a key function 
instead, or left as is?
msg77261 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-12-07 21:30
On Sun, Dec 7, 2008 at 13:26, Mark Dickinson <report@bugs.python.org> wrote:
>
> Mark Dickinson <dickinsm@gmail.com> added the comment:
>
> About unittest:
>
> unittest.TestLoader has an attribute "sortTestMethodsUsing", which it
> expects to be an old-style comparison.
>
> Should this attribute be updated (and renamed?) to expect a key function
> instead, or left as is?
>

It will break backwards-compatibility if you change it. Possibly the
best solution is to introduce a new attribute that does either key or
just lt comparison and make sortTestMethodsUsing be a descriptor that
wraps the function as needed.
msg77281 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-08 01:48
The latest patch removes PyUnicode_Compare as well as lots of __cmp__
functions under Lib/. It also renames and redefines
PyUnicode_CompareWithASCIIString(). The simpler
PyUnicode_EqualToASCIIString() function is easier to use, too.
msg77297 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-08 11:23
Update patch to include fix for unittest.py and test_unittest.py.
msg77311 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-08 15:35
I've created a new branch to work on the issue:
svn+ssh://pythondev@svn.python.org/python/branches/py3k-issue1717. It's
easier to work on a branch than exchanging monster patches.
msg77313 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-08 15:53
Please put the PyUnicode_Compare() API back in there.

Removing __cmp__ really doesn't have anything to do with removing often
used helper functions for comparing certain object types and only
cripples the C API in a needless way.

Thanks.
msg77345 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-08 21:30
Instead of removing cmp(a, b) and replacing all uses with (a>b)-(b<a) I
think it's better to turn cmp() into a helper that applies this
operation in C rather than Python.
msg77361 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-12-08 23:12
Marc-Andre, how  do you like a function PyUnicode_Equal() instead of
PyUnicode_Compare()?

The old PyUnicode_Compare() function returns -1 for smaller value and
errors, 0 for equality and +1 for larger value. I find it confusing to
have one function that follows the old PyObject_Compare() behavior.

Instead I'm proposing PyUnicode_Equal() which returns -1 for errors, 0
for unequal and +1 for equal. The function follows the semantic of
http://docs.python.org/c-api/object.html#PyObject_RichCompareBool nicely.

About your proposal for cmp(), where should we put the method? I'm -0.5
on builtins.
msg77371 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-08 23:52
IMO PyUnicode_Compare() should be replaced by a hypothetical
PyObject_RichCompare(), which allows to take shortcuts when comparing
strings of different length and a Py_EQ or Py_NE comparison is requested.
(see #3106 too)
msg77372 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-08 23:53
On 2008-12-09 00:12, Christian Heimes wrote:
> Christian Heimes <lists@cheimes.de> added the comment:
> 
> Marc-Andre, how  do you like a function PyUnicode_Equal() instead of
> PyUnicode_Compare()?
> 
> The old PyUnicode_Compare() function returns -1 for smaller value and
> errors, 0 for equality and +1 for larger value. I find it confusing to
> have one function that follows the old PyObject_Compare() behavior.

For Unicode objects you don't have the issues with general rich
comparisons. Unicode objects only know <, = and >, so the C approach
of using -1, 0, +1 for these results will work just fine.

It doesn't in the general case, where comparisons can return arbitrary
objects.

> Instead I'm proposing PyUnicode_Equal() which returns -1 for errors, 0
> for unequal and +1 for equal. The function follows the semantic of
> http://docs.python.org/c-api/object.html#PyObject_RichCompareBool nicely.

Yes, but for sorting Unicode (in C) and other quick checks that rely on
lexical ordering you need checks for < and > as well.

It's better to have just one C API than 3 or 5 to cover all comparison
cases.

> About your proposal for cmp(), where should we put the method? I'm -0.5
> on builtins.

Good question.

It is in the builtins in Python 2.x, so keeping it there would make
porting easier, but then it's probably better to put it somewhere else
to make people aware of the fact that the underlying machinery has
changed.
msg77374 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-08 23:58
I wrote:
> IMO PyUnicode_Compare() should be replaced by a hypothetical
> PyObject_RichCompare(), which allows to take shortcuts when comparing
> strings of different length and a Py_EQ or Py_NE comparison is requested.

... and I didn't even remember that PyUnicode_RichCompare() already
exists. So let my proposal be to enable those optimizations inside the
existing PyUnicode_RichCompare(), and dump PyUnicode_Compare().
msg77375 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-09 00:10
On 2008-12-09 00:58, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> I wrote:
>> IMO PyUnicode_Compare() should be replaced by a hypothetical
>> PyObject_RichCompare(), which allows to take shortcuts when comparing
>> strings of different length and a Py_EQ or Py_NE comparison is requested.
> 
> ... and I didn't even remember that PyUnicode_RichCompare() already
> exists. So let my proposal be to enable those optimizations inside the
> existing PyUnicode_RichCompare(), and dump PyUnicode_Compare().

What for ? What does this have to do with removing __cmp__ ?
Why do you want to break the C API for no apparent reason ?

I've designed the Unicode C API to be a rich API and would like
it to stay that way.
msg77388 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-09 02:47
> It is in the builtins in Python 2.x, so keeping it there would make
> porting easier, but then it's probably better to put it somewhere else
> to make people aware of the fact that the underlying machinery has
> changed.

from __past__ import cmp

?
msg77396 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-09 09:22
More seriously, if cmp were to go into the standard library somewhere,
perhaps Raymond's class decorator (for filling in missing rich
comparisons) could go into the same place?
msg77398 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-09 09:59
Moving cmp() somewhere other than builtins is not progress.  IMO, it
needs to die off and the concept of it needs to disappear completely. 
Code is better without it.  Three-way comparisons are PITA to use --
their only virtue is as an optimization in the few cases where one
three-way test is cheaper to compute than two two-way tests.

The goal for 3.0 was to have one way to do it and thereby simplify the
language.  Tucking it away in a module or writing a new decorator
defeats the purpose.  It simply should die and go away.
msg77403 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-09 11:53
On 2008-12-09 10:59, Raymond Hettinger wrote:
> Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment:
> 
> Moving cmp() somewhere other than builtins is not progress.  IMO, it
> needs to die off and the concept of it needs to disappear completely. 
> Code is better without it.  Three-way comparisons are PITA to use --
> their only virtue is as an optimization in the few cases where one
> three-way test is cheaper to compute than two two-way tests.
> 
> The goal for 3.0 was to have one way to do it and thereby simplify the
> language.  Tucking it away in a module or writing a new decorator
> defeats the purpose.  It simply should die and go away.

The idea was to have one implementation of the work-around (a>b) - (b<a)
instead of 10 or so instances of this snippet in the Python stdlib
and probably a few hundred places in other code.

Indeed, the motivation is to have one obvious way to write this
work-around :-)

Note that cmp() doesn't make __cmp__ come back, but it does help
porting code that uses cmp().

Besides, cmp() is available builtin in 3.0, so it's too late to just
remove it anyway. We'd have to go through the usual deprecation
process.
msg77420 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-09 15:06
> The idea was to have one implementation of the work-around (a>b) - (b<a)
> instead of 10 or so instances of this snippet in the Python stdlib
> and probably a few hundred places in other code.

But what use-case does it solve, except for making legacy code easier to
port to Py3k?
msg77434 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-09 16:45
On 2008-12-09 16:06, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
>> The idea was to have one implementation of the work-around (a>b) - (b<a)
>> instead of 10 or so instances of this snippet in the Python stdlib
>> and probably a few hundred places in other code.
> 
> But what use-case does it solve, except for making legacy code easier to
> port to Py3k?

It implements the DRY principle.
msg77444 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-12-09 20:26
Guido approved removing __builtin__.cmp() in 3.0.1.  It was supposed to
have been taken out but was forgotten.

With respect to the DRY principle, I disagree about its utility here. 
The code is so simple that it doesn't warrant cross-module
factorization.  It is a better goal to minimize dependencies.
msg78091 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-20 11:00
Still to do:

pybench needs updating to remove a cmp reference;  since the changes 
required for pybench are a little bit more substantial than simple cmp 
replacement, I've broken this out into a separate issue:  issue 4704.

There are many uses of cmp still in the Demos directory.  How much do we 
care?

The documentation in Doc/extending/newtypes.rst needs updating.  I can 
have a go at this, but Georg would almost certainly do a better job.
msg78092 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-12-20 11:18
Should Py_CmpToRich (in object.c) disappear?

It's currently used in longobject.c, but nowhere else as far as I can 
tell.  It shouldn't be too hard to update longobject.c to remove the use.
msg78095 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-20 12:46
> It shouldn't be too hard to update longobject.c to remove the use.

I'll do that one.
msg78101 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-20 13:26
> > It shouldn't be too hard to update longobject.c to remove the use.
> 
> I'll do that one.

Done in r67871, r67873.
msg78110 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-12-20 15:16
Please remember to add back PyUnicode_Compare() if that hasn't already
been done.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com

________________________________________________________________________
2008-12-02: Released mxODBC.Connect 1.0.0      http://python.egenix.com/

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
msg78813 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-02 15:09
Here's a patch (against py3k) generated from the current state of the 
py3k-issue1717 branch, for ease of review and testing.

The patch needs serious review;  it should be considered a first draft, 
and there are probably many more changes still to be made.  I don't 
think I can do much more for now without getting input from others.

Known places in the source tree where cmp/__cmp__ still lingers on:

Demos/<many>
Doc/extending/newtypes.rst
Misc/cheatsheet
Misc/python-mode.el
Misc/Vim/python.vim
Parser/spark.py   # (I don't know what this does.  Anyone?)
Tools/<various>   # (notably pynche and pybench)

Apart from the newtypes.rst, all of the above files are somewhat out of 
date in other ways.  In my opinion it's only the doc fixes that stop the 
patch from being complete enough.
msg78853 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-01-02 18:11
Doc/extending/newtypes.rst

  I'll fix this one after the patch has landed.

Misc/cheatsheet

  This needs a general overhaul.

Misc/python-mode.el

  I think this should be removed from the distribution; it's maintained
externally.

Parser/spark.py   # (I don't know what this does.  Anyone?)

  This is used by asdl_c.py which generates Python-ast.c -- it should be
updated.
msg78953 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-03 12:48
[Georg Brandl, on spark.py]
> This is used by asdl_c.py which generates Python-ast.c -- it should be
> updated.

The only issue here is a single comment, which reads:

#  GenericASTMatcher.  AST nodes must have "__getitem__" and "__cmp__"

Still, I'm reluctant to remove or alter the comment without
understanding why it was there in the first place.  If someone more
familiar with AST stuff can confirm that __cmp__ is definitely no
longer needed, I'll remove the mention of it from the comment.

> Misc/cheatsheet
>
>  This needs a general overhaul.

I'll open a separate issue for this, if that's okay with everyone.  
Presumably 3.0.1 doesn't need to wait for Misc/cheatsheet to be updated.
msg80503 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-25 12:14
I'm wondering how to move forward with this issue.  Might it make sense to 
break the current monster patch into a series of more-easily reviewed 
patches?  I was thinking of something like:

 - patch 1 (Python code): remove all uses of cmp from std. lib. and tests
 - patch 2 (C code): remove all uses of tp_compare from Modules and 
Objects
 - patch 3 : rename tp_compare to tp_reserved in all Modules and Objects 
(should be a very simple patch, but touches a lot of files)
 - patch 4 : fix docs, and look for any other loose ends that need
   cleaning up.

Here's patch 1: remove uses of cmp from library and tests (and also from 
Tools/unicode/makeunicodedata.py).  It involves essentially no user-
visible changes.  (At least, assuming that users aren't trying to access 
e.g., UserString.__cmp__ directly.)

The patch is against py3k.  All tests pass on my machine with this patch 
applied.

Anyone interested in reviewing this?
msg80504 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-25 12:38
Quick comments on your patch:
- two files have unwanted modifications on non-ASCII characters
(Lib/heapq.py and Lib/sqlite3/test/hooks.py)
- you haven't renamed "__cmp__" to "_cmp" in Lib/xmlrpc/client.py:
deliberate?
- in Lib/heapq.py, there's a "_cmp__" which should be "_cmp"
msg80505 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-25 13:00
Thanks, Antoine.  Here's a new patch.

> - two files have unwanted modifications on non-ASCII characters
> (Lib/heapq.py and Lib/sqlite3/test/hooks.py)

Fixed, I think.  Could you double check?

> - you haven't renamed "__cmp__" to "_cmp" in Lib/xmlrpc/client.py:
> deliberate?

Not deliberate!  This __cmp__ should simply be removed, I think---the rich 
comparison methods are already present.

> - in Lib/heapq.py, there's a "_cmp__" which should be "_cmp"

Replaced "_cmp__" (which was in a comment) with "comparison".
msg80511 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-25 15:48
> Fixed, I think.  Could you double check?

It's ok!
msg80667 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-27 18:31
Stage 1 committed in r69025 (py3k) and r69026 (release30-maint).
msg80699 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-28 08:53
Can anyone who uses tkinter give me some advice?  Does PyTclObject in 
_tkinter.c need to have its tp_richcompare method implemented?  And if so, 
how do I go about testing the implementation?  It seems that PyTclObjects 
aren't directly exposed to Python under 'import tkinter'.

I'll hold off on any more checkins until the 3.0.1 thread on python-dev 
has resolved itself.
msg80706 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2009-01-28 12:53
Mark,

I'm not a very huge user of tkinter, but I can tell you it would be
tricky to try getting a PyTclObject. It needs to be exposed if you want
to test it without relying on Tcl, but, to me they are just a
"temporary" object that serves to indicate the caller that it needs to
be converted to something else since _tkinter itself wasn't able to do
it. For this reason I would actually prefer to them not be comparable.
msg80709 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-28 13:37
Thanks, Guilherme.

> For this reason I would actually prefer to them not be comparable.

That's fine with me, so long as we can be sure that there's no existing 
code that depends on them being comparable.  I can't figure out whether 
there's any legitimate way that the tp_compare slot (which is currently 
implemented) of a PyTclObject could ever be called.
msg80751 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-29 09:22
I'm not going to get more time to work on this before
the weekend, so if anyone else wants to take over please
feel free.

Still to do for stage 2:  cell objects and slot
wrapper objects need to have tp_richcompare
implemented, to replace the tp_compare slots that
are being removed.
msg80752 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-01-29 09:38
For 3.0, are you going to keep tp_compare slot in existence and just
assert that it is NULL?  Then in 3.1, remove the slot entirely?
msg80753 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-29 09:45
If I understand Christian's plan correctly, it was to:

(1) raise TypeError for non-NULL tp_compare, and
(2) rename tp_compare to tp_reserved (with type void *).

and both of these would happen with 3.0.1, so no difference between
3.0.1 and 3.1.0.

It seems to me that if tp_compare is actually going to be removed then
that should be done in 3.0.1;  else third-party stuff that works with
3.0.x will fail with 3.1, due to the various tp_* fields being out of sync.

It would be nice not to have tp_reserved hanging around for the duration
of 3.x.  (Similarly for nb_reserved.)
msg80754 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-01-29 09:53
Instead of tp_reserved, the name should be tp_deprecated_compare.

There should be a python-dev discussion around when to actually remove
the slot:

  3.0.1 -- binary incompatibility between minor releases (BAD)
  3.1.0 -- uncomfortable for writers who have to add #ifdefs
  4.0.0 -- no pain, just wasted space
  3.1.5 -- just plain mean :-)
msg80770 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-29 15:30
Actually, I would like to repurpose tp_compare as tp_bytes for the
__bytes__ method.
msg80780 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-01-29 19:39
On Thu, Jan 29, 2009 at 07:30, Benjamin Peterson <report@bugs.python.org> wrote:
>
> Benjamin Peterson <benjamin@python.org> added the comment:
>
> Actually, I would like to repurpose tp_compare as tp_bytes for the
> __bytes__ method.

Repurposing would be extremely bad as that would mean it is possible
for people to actually compile code with a busted __bytes__
implementation.
msg80781 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-29 19:40
On Thu, Jan 29, 2009 at 2:39 PM, Brett Cannon <report@bugs.python.org> wrote:
>
> Brett Cannon <brett@python.org> added the comment:
>
> On Thu, Jan 29, 2009 at 07:30, Benjamin Peterson <report@bugs.python.org> wrote:
>>
>> Benjamin Peterson <benjamin@python.org> added the comment:
>>
>> Actually, I would like to repurpose tp_compare as tp_bytes for the
>> __bytes__ method.
>
> Repurposing would be extremely bad as that would mean it is possible
> for people to actually compile code with a busted __bytes__
> implementation.

Wasn't there another case of an old slot being reused?
msg80782 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-01-29 19:54
>> On Thu, Jan 29, 2009 at 07:30, Benjamin Peterson <report@bugs.python.org> wrote:
>>>
>>> Benjamin Peterson <benjamin@python.org> added the comment:
>>>
>>> Actually, I would like to repurpose tp_compare as tp_bytes for the
>>> __bytes__ method.
>>
>> Repurposing would be extremely bad as that would mean it is possible
>> for people to actually compile code with a busted __bytes__
>> implementation.
>
> Wasn't there another case of an old slot being reused?

Not that I can remember, but I'm sure someone will correct me if I'm wrong.
msg80830 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-30 17:38
Here's stage 2:  remove uses of tp_compare from Objects and Modules, and
replace uses of PyObject_Compare with PyObject_RichCompareBool.
PyObject_Compare, cmp and friends still haven't been removed at this
stage.

In detail:
  - for cell objects, method wrapper objects, PyTclObjects, and
    PyST_Objects (in the parser module), remove the defined tp_compare
    methods and implement tp_richcompare instead.
  - add tests for cell comparisons and PyST_Object comparisons;  
reenable
    tests for method wrapper comparisons.  There are no tests for the
    PyTclObject comparisons.
  - remove tp_compare method from sets (all it did was emit an error
    message about the nonsensicality of doing order comparisons on sets)
  - in Objects/rangeobject.c and ElementTree, replace uses of
    PyObject_Compare with PyObject_RichCompareBool
msg80843 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-01-31 00:31
I haven't stared very closely but it looks ok.
("spanish armada" might be replaced with "spanish inquisition", though)
msg80907 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-01 12:35
Thanks for the review, Antoine.

Stage 2 applied to py3k in r69181, merged to 3.0 in r69182.

cmp, PyObject_Cmp and PyObject_Compare removed in r69184 (py3k) and r69185 
(release30-maint).

There's still the rename of the tp_compare slot to deal with, and a fair 
amount of cleaning up and documentation fixing to do.
msg81008 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-02 22:08
All relevant changes from the py3k-issue1717 branch have now been merged into the py3k 
branch (and from there into the 3.0 maintenance branch), in a series of revisions.  
Here they are, listed in py3k/release30-maint pairs:

r69188, r69189,
r69190, r69191,
r69192, r69193,
r69214, r69215,
r69218, r69221,
r69224, r69225.

The idea to raise TypeError on non-NULL tp_compare was abandoned after Martin pointed 
out that it would be a binary incompatible change _[1], but Nick Coghlan suggested a 
DeprecationWarning when tp_compare is non-NULL and tp_richcompare is NULL _[2], and 
remarked that the warning should also be implemented for Python 2.7 when run with the -
3 option.

Still to do before this can be closed:

 - fix up Doc/extending/newtypes.rst;  I think Georg has said he'll
   take care of this, so once everything else is done I'll just reassign
   this issue to him and let him get on with it.

 - add Nick Coghlan's suggested DeprecationWarning.

 - grep through the sources looking for tp_compare, __cmp__, cmpfunc,
   PyObject_Cmp, PyObject_Compare and cmp, and check all remaining
   references are legitimate.

 - update pybench;  there's a separate issue already open for this.
   (issue 4704), and it looks like Antoine and Marc-André are on the
   case.

 - anything else that I've forgotten.

Thanks everyone for your help so far, especially Christian for much of
the original code and Antoine for code and review.

[1] http://mail.python.org/pipermail/python-dev/2009-February/085797.html
[2] http://mail.python.org/pipermail/python-dev/2009-February/085809.html
msg81373 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-08 11:10
Deprecation warning for types that implement tp_compare but not 
tp_richcompare added in r69431, r69432.

Just the doc fixes in Doc/extending/newtypes.rst left.  Assigning to Georg  
and reducing priority.
msg82258 - (view) Author: Anthony Tuininga (atuining) * Date: 2009-02-16 18:14
Removing cmp() breaks distutils. I get the following exception, for
example using the just released version 3.0.1:

Traceback (most recent call last):
  File "setup.py", line 318, in <module>
    classifiers = classifiers)
  File "c:\Python30\lib\distutils\core.py", line 149, in setup
    dist.run_commands()
  File "c:\Python30\lib\distutils\dist.py", line 942, in run_commands
    self.run_command(cmd)
  File "c:\Python30\lib\distutils\dist.py", line 962, in run_command
    cmd_obj.run()
  File "c:\Python30\lib\distutils\command\build.py", line 128, in run
    self.run_command(cmd_name)
  File "c:\Python30\lib\distutils\cmd.py", line 317, in run_command
    self.distribution.run_command(command)
  File "c:\Python30\lib\distutils\dist.py", line 962, in run_command
    cmd_obj.run()
  File "c:\Python30\lib\distutils\command\build_ext.py", line 306, in run
    force=self.force)
  File "c:\Python30\lib\distutils\ccompiler.py", line 1110, in new_compiler
    return klass(None, dry_run, force)
  File "c:\Python30\lib\distutils\cygwinccompiler.py", line 314, in __init__
    if self.gcc_version <= "2.91.57":
  File "c:\Python30\lib\distutils\version.py", line 64, in __le__
    c = self._cmp(other)
  File "c:\Python30\lib\distutils\version.py", line 341, in _cmp
    return cmp(self.version, other.version)
NameError: global name 'cmp' is not defined
msg82260 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-02-16 18:25
Fixed in r69682.
msg82262 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-16 18:30
Darn.  That's really very annoying.  Apologies for missing this one.

Thanks for the quick fix, Benjamin.
msg84859 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-03-31 18:56
Documentation updated in r70863.
msg195849 - (view) Author: Roundup Robot (python-dev) Date: 2013-08-22 01:14
New changeset 64e004737837 by R David Murray in branch '3.3':
#18324: set_payload now correctly handles binary input.
http://hg.python.org/cpython/rev/64e004737837
History
Date User Action Args
2013-08-22 01:14:27python-devsetnosy: + python-dev
messages: + msg195849
2009-03-31 18:56:48georg.brandlsetstatus: open -> closed
resolution: fixed
messages: + msg84859
2009-03-05 20:23:40benjamin.petersonsetpriority: release blocker -> normal
2009-02-16 18:30:17mark.dickinsonsetmessages: + msg82262
2009-02-16 18:25:20benjamin.petersonsetmessages: + msg82260
2009-02-16 18:23:06georg.brandlsetpriority: normal -> release blocker
2009-02-16 18:14:42atuiningsetnosy: + atuining
messages: + msg82258
2009-02-08 11:10:47mark.dickinsonsetpriority: release blocker -> normal
assignee: georg.brandl
messages: + msg81373
versions: + Python 3.1
2009-02-02 22:08:57mark.dickinsonsetmessages: + msg81008
2009-02-01 12:35:05mark.dickinsonsetmessages: + msg80907
2009-01-31 00:31:26pitrousetmessages: + msg80843
2009-01-30 17:38:48mark.dickinsonsetfiles: + 1717_stage2.patch
messages: + msg80830
2009-01-29 19:54:18brett.cannonsetmessages: + msg80782
2009-01-29 19:40:25benjamin.petersonsetmessages: + msg80781
2009-01-29 19:39:20brett.cannonsetmessages: + msg80780
2009-01-29 15:30:06benjamin.petersonsetmessages: + msg80770
2009-01-29 09:53:01rhettingersetmessages: + msg80754
2009-01-29 09:45:23mark.dickinsonsetmessages: + msg80753
2009-01-29 09:38:38rhettingersetmessages: + msg80752
2009-01-29 09:22:19mark.dickinsonsetmessages: + msg80751
2009-01-28 13:37:09mark.dickinsonsetmessages: + msg80709
2009-01-28 12:53:12gpolosetnosy: + gpolo
messages: + msg80706
2009-01-28 08:53:15mark.dickinsonsetmessages: + msg80699
2009-01-27 18:31:43mark.dickinsonsetmessages: + msg80667
2009-01-25 15:48:57pitrousetmessages: + msg80511
2009-01-25 13:00:10mark.dickinsonsetfiles: + 1717_stage1_v2.patch
messages: + msg80505
2009-01-25 12:38:03pitrousetmessages: + msg80504
2009-01-25 12:14:35mark.dickinsonsetfiles: + 1717_stage1.patch
messages: + msg80503
2009-01-03 12:55:43loewissettitle: Get rid of more refercenes to __cmp__ -> Get rid of more references to __cmp__
2009-01-03 12:48:19mark.dickinsonsetmessages: + msg78953
2009-01-02 18:11:07georg.brandlsetmessages: + msg78853
2009-01-02 15:10:04mark.dickinsonsetfiles: + remove_cmp6.patch
messages: + msg78813
stage: needs patch -> patch review
2008-12-20 15:16:39lemburgsetmessages: + msg78110
2008-12-20 13:26:21pitrousetmessages: + msg78101
2008-12-20 12:46:07pitrousetmessages: + msg78095
2008-12-20 11:18:05mark.dickinsonsetmessages: + msg78092
2008-12-20 11:00:32mark.dickinsonsetdependencies: + Update pybench for python 3.0
messages: + msg78091
2008-12-20 02:41:00loewissetpriority: deferred blocker -> release blocker
2008-12-10 08:25:12loewissetpriority: release blocker -> deferred blocker
2008-12-09 20:26:46rhettingersetmessages: + msg77444
2008-12-09 16:45:28lemburgsetmessages: + msg77434
2008-12-09 15:06:54pitrousetmessages: + msg77420
2008-12-09 11:53:18lemburgsetmessages: + msg77403
2008-12-09 09:59:18rhettingersetmessages: + msg77398
2008-12-09 09:22:45mark.dickinsonsetmessages: + msg77396
2008-12-09 02:47:00mark.dickinsonsetmessages: + msg77388
2008-12-09 00:10:35lemburgsetmessages: + msg77375
2008-12-08 23:58:53pitrousetmessages: + msg77374
2008-12-08 23:53:22lemburgsetmessages: + msg77372
2008-12-08 23:52:48pitrousetnosy: + pitrou
messages: + msg77371
2008-12-08 23:12:28christian.heimessetmessages: + msg77361
2008-12-08 21:38:51gvanrossumsetnosy: - gvanrossum
2008-12-08 21:30:01lemburgsetmessages: + msg77345
2008-12-08 15:53:17lemburgsetnosy: + lemburg
messages: + msg77313
2008-12-08 15:35:46christian.heimessetmessages: + msg77311
2008-12-08 11:23:49mark.dickinsonsetfiles: + remove_cmp5.patch
messages: + msg77297
2008-12-08 01:48:44christian.heimessetfiles: + remove_cmp4.patch
messages: + msg77281
stage: needs patch
2008-12-07 21:30:30brett.cannonsetmessages: + msg77261
2008-12-07 21:26:45mark.dickinsonsetmessages: + msg77258
2008-12-07 21:03:54mark.dickinsonsetfiles: + remove_cmp3.patch
messages: + msg77255
2008-12-07 18:15:10christian.heimessetfiles: + remove_cmp2.patch
messages: + msg77239
2008-12-07 18:13:25christian.heimessetfiles: - remove_cmp.patch
2008-12-07 17:31:22mark.dickinsonsetfiles: + remove_cmp_decimal.patch
messages: + msg77237
2008-12-07 17:29:46christian.heimessetmessages: + msg77236
2008-12-07 17:26:25mark.dickinsonsetmessages: + msg77235
2008-12-07 16:11:23christian.heimessetfiles: + remove_cmp.patch
messages: + msg77231
2008-12-07 12:36:55christian.heimessetmessages: + msg77222
2008-12-07 12:19:26mark.dickinsonsetmessages: + msg77220
2008-12-07 01:58:14rhettingersetmessages: + msg77207
2008-12-07 01:50:54rhettingersetmessages: + msg77206
2008-12-07 01:14:24rhettingersetmessages: + msg77203
2008-12-07 00:23:11amaury.forgeotdarcsetmessages: + msg77199
2008-12-06 23:47:04benjamin.petersonsetmessages: + msg77192
2008-12-06 23:45:18amaury.forgeotdarcsetmessages: + msg77191
2008-12-06 23:39:00amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg77189
2008-12-06 23:33:47barrysetpriority: critical -> release blocker
2008-12-06 23:24:01gvanrossumsetmessages: + msg77187
2008-12-06 23:04:07christian.heimessetnosy: + christian.heimes
messages: + msg77184
2008-12-06 22:58:14rhettingersetmessages: + msg77182
2008-12-06 22:49:53rhettingersetnosy: + rhettinger
2008-12-06 22:15:51georg.brandlsetmessages: + msg77175
2008-12-06 22:12:53gvanrossumsetmessages: + msg77174
2008-12-06 22:09:10georg.brandlsetmessages: + msg77173
2008-12-06 16:42:15gvanrossumsetmessages: + msg77143
2008-12-06 08:15:40georg.brandlsetpriority: high -> critical
resolution: fixed -> (no value)
messages: + msg77105
2008-12-06 08:15:05georg.brandllinkissue4556 superseder
2008-10-16 20:55:49benjamin.petersonsetpriority: release blocker -> high
messages: + msg74857
2008-10-16 20:33:06mark.dickinsonsetmessages: + msg74854
2008-10-16 19:37:06georg.brandlsetstatus: closed -> open
messages: + msg74849
2008-10-16 19:35:12benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg74848
2008-10-16 07:20:28mark.dickinsonsetfiles: + nocmp_tests.diff
nosy: + mark.dickinson
messages: + msg74829
2008-10-02 12:54:30barrysetpriority: deferred blocker -> release blocker
2008-09-26 22:18:51barrysetpriority: release blocker -> deferred blocker
2008-09-18 05:43:28barrysetpriority: deferred blocker -> release blocker
2008-09-10 02:41:00brett.cannonsetpriority: release blocker -> deferred blocker
nosy: + brett.cannon
messages: + msg72939
2008-09-09 22:01:05benjamin.petersonsetmessages: + msg72924
2008-09-09 20:12:59georg.brandlsetpriority: critical -> release blocker
messages: + msg72903
2008-08-07 18:15:38georg.brandlsetmessages: + msg70846
2008-08-05 16:00:56gvanrossumsetassignee: gvanrossum -> (no value)
messages: + msg70745
2008-07-31 02:06:56benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg70472
2008-03-29 01:30:40georg.brandlsetpriority: normal -> critical
nosy: + georg.brandl
messages: + msg64668
2008-03-18 05:07:21gvanrossumsetpriority: low -> normal
messages: + msg63894
2008-01-01 16:39:25gvanrossumcreate