This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: __bool__ instead of __nonzero__
Type: Stage:
Components: Interpreter Core Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: jackdied Nosy List: doerwalter, gangesmaster, georg.brandl, jackdied
Priority: normal Keywords: patch

Created on 2006-11-21 11:57 by gangesmaster, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
bool4.patch gangesmaster, 2006-11-21 21:33 new patch
bool5.patch doerwalter, 2006-11-22 18:55
bool6.patch jackdied, 2006-11-23 00:13
Messages (17)
msg51385 - (view) Author: ganges master (gangesmaster) Date: 2006-11-21 11:57
this patch replaces __nonzero__ with __bool__, to make bool type symmetrical to all other types (__int__, __float__, etc.)

some notes:
* the latex docs have been updated
* the slot name was changed from nb_nonzero to nb_bool
* all XXX_nonzero methods where changed to XXX_bool
* all the comments relating to nb_nonzero have been updated
* stdlib was updated
* all the test suites were updated

otoh, i couldn't get it to compile (MSVC++2003), because of a strange bug in ceval.c (none of my code). seems the GETLOCAL macro causes syntactic problems, but i didn't have time to check it thoroughly.
msg51386 - (view) Author: ganges master (gangesmaster) Date: 2006-11-21 18:28
* slot_nb_bool now requires __bool__ to return only a boolean
* tab-width issues (hopefully) fixed
msg51387 - (view) Author: ganges master (gangesmaster) Date: 2006-11-21 21:33
fixed a bug with type checking when using __len__
msg51388 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2006-11-22 18:55
bool5.patch removes the check for int for the __len__() result as this is already done in the __len__() call itself. It adds a few tests to test_bool.py::BoolTest.test_convert_to_bool() and fixes the description of __bool__ in Doc/ref/ref3.tex.
msg51389 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-22 19:58
I'm not convinced slot_nb_bool is doing the right thing in 
the PyBool_Check(tmp) line.  There used to be a "result = -1;"
right after the PyErr_Format and there isn't anymore (maybe
result gets set to -1 somewhere else but I can't tell where).

Can you undo the refactoring of that function in general?
Some of the decrefs moved around too and they look correct
but it would be easier to tell if they just stayed the same.

Also, did you mean to reindent the long_as_number struct
in longobject.c?
msg51390 - (view) Author: ganges master (gangesmaster) Date: 2006-11-22 20:07
> There used to be a "result = -1;"
result is initialized to -1 at the beginning of the function

> did you mean to reindent the long_as_number struct in longobject.c?
i realigned the struct with tab of 4, where it used to be tabs of 8,
so things got messed up a little. i tried my best to fix it, but 
i can't fix what i can't see :)

> bool5.patch removes the check for int for the __len__() result as this is
> already done in the __len__() call itself
no it's not! we are not using PyObject_Len, we directly invoke __len__, 
which may return anything it wishes. you can't skip that check.

msg51391 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-22 20:30
Ah, I missed the default of -1

The refactoring changed how the results of lookup_maybe()s
are used.  From the note at the top of lookup_maybe() it
might return NULL and _not_ set an exception.  In the old
code it checked for NULL versus PyErr_Occcurred.  In the
new code it looks like func will get decrefed if it is NULL
but PyErr_Occurred is not set.  I would revert the refactoring
and change only the bits you have to.

[I'll pass on the __len__ thing until I have a chance to
look at it more]
msg51392 - (view) Author: ganges master (gangesmaster) Date: 2006-11-22 20:43
> In the
> new code it looks like func will get decrefed if it is NULL
> but PyErr_Occurred is not set.

true. i neglected that :)
msg51393 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-22 23:35
I don't have sf permissions so I put a tweaked version of the patch
at http://jackdied.com/static/bool6.patch

* keeps slots_nb_bool closer to the original
* extra test in test_bool to exercise __len__ fallbacks
* test_iter and test_decimal pass
* reverted longobject.c indentation

The regular __len__ machinery does get called but the 2.5 code does
the bool/int check either way because it made the code shorter
(both __len__ and __nonzero__ could return an int so doing
the same check for both didn't hurt anything).  The new patch
treats the result of __len__ and __bool__ slightly differently
because __bool__ must return __bool__.  I added a couple lines
but otherwise left the function as it was (so the lookup_maybe()s
should be fine)

If there are no objections I'll check it in.
msg51394 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-11-22 23:47
Applies and compiles fine here. I was about to comment about a refleak somewhere (I ran test_iter with -R and it showed 222 leaked refs), but it shows up without the patch too.

I'll update PEP 3100 after you checked it in, so that people writing the conversion tool/advisor will remember to include this one.
msg51395 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-11-22 23:55
Applies and compiles fine here. I was about to comment about a refleak somewhere (I ran test_iter with -R and it showed 222 leaked refs), but it shows up without the patch too.

I'll update PEP 3100 after you checked it in, so that people writing the conversion tool/advisor will remember to include this one.
msg51396 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-23 00:15
Thanks to the [ever vigilant] gbrandl I have sf permissions so the patch is now attached below.
msg51397 - (view) Author: ganges master (gangesmaster) Date: 2006-11-24 08:57
well, i liked it better refactored, i.e., each piece of code
taking care of one logical part (i.e., one section for __bool__,
another for __len__, and another for the default.
now it's back to the spaghetti it was.

> return PyErr_Occurred() ? -1 : 1;
i hate the trinary expression, but that's all about style :)

anyway, as long as it works, i don't have any complaints.
you just have to make sure it's written as optimized as possible,
because it gets invoked many times behind the scenes.
msg51398 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2006-11-27 18:11
According to http://coverage.livinglogic.de/Objects/typeobject.c.html the code in typeobject.c::slot_nb_nonzero() (for the 2.6 version) gets executed ca. 270000 time during the run of the test suite (compared to e.g. 5700000000 (5.7e9) for the most executed code in ceval.c). So I don't think that performance optimization is *that* critical.
msg51399 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-28 01:30
intobject.c:int_nonzero() gets called 100 times more than that
in the same test run.  Actually all builtins go through the 
fast slots machinery and not slot_nb_nonzero() so I'm not 
worried about speed (if there is any to be had).

I tried adding a nb_bool to listobject.c (currently it falls
back on tp_as_mapping->mp_length) and I couldn't tell the
difference with 
./python Lib/timeit.py "if [] or [] or [] or [] or [] or [] or [] or [] or []: pass"
So it looks like the places that matter are already fast.
msg51400 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2006-11-28 15:35
OK, so if there are no other objections go ahead and check it in.
msg51401 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2006-11-28 19:20
Committed revision 52853
History
Date User Action Args
2022-04-11 14:56:21adminsetgithub: 44262
2008-01-06 22:29:46adminsetkeywords: - py3k
versions: + Python 3.0
2006-11-21 11:57:30gangesmastercreate