Francisco Martín Brugué added the comment:
What we want to test is that the return value is of type 'int', which is what
Victor's test checks.
Thank you for the explanations!
for 2.7.6 type(2 62) is type 'long' and type(2 61) is
type 'int' (I suppose it's analogous in a 32 bit machine
Francisco Martín Brugué added the comment:
Hi,
sorry if it's trivial but shouldn't we add a 'shifted_true' test
some were to make sure that this behavior change gets noticed next time?
def test_shifted_true(self):
self.assertEqual(True 0, 1)
self.assertEqual(True 1, 2)
--
R. David Murray added the comment:
Francisco: I'd say that was a good idea. Would you like to propose a patch?
(ie: figure out where it should go)
--
nosy: +r.david.murray
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
Arfrever Frehtes Taifersar Arahesis added the comment:
Use assertIs, since True == 1, but True is not 1.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
___
Roundup Robot added the comment:
New changeset ef49aaad3812 by Victor Stinner in branch '3.4':
Issue #21422: Add a test to check that bool int and bool int return an int
http://hg.python.org/cpython/rev/ef49aaad3812
New changeset 3da4aed1d18a by Victor Stinner in branch 'default':
(Merge 3.4)
STINNER Victor added the comment:
Ok, 3 core dev are opposed to the change, I close the issue.
I added a test on bool int and bool int to ensure that the result is an
int.
--
resolution: - rejected
status: open - closed
___
Python tracker
Francisco Martín Brugué added the comment:
I Victor you were so fast, I started with one patch also in bool (at least the
place was right). The problem is that I was getting some extrage (for me at
least). As far I hat:
def test_shifted_true(self):
with
STINNER Victor added the comment:
What I was doing wrong?
The is operator should only be used to compare identical objects. Small
integers (range -5..255 if I remember correctly) are singletons. I prefer to
not rely on this implementation detail in a unit test of the Python standard
R. David Murray added the comment:
Arfrever's advice was misleading...the test would have needed to be
assertIsNot(True 0, 1), but the fact that True is not preserved is not
really what we want to test. What we want to test is that the return value is
of type 'int', which is what Victor's
R. David Murray added the comment:
Arfrever pointed out on irc that I misread. It would indeed be assertIs(True
0, 1), but that wouldn't make a good test because the fact that '1 is 1' is
an implementation detail of CPython and can't be relied upon.
--
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:
--
nosy: +Arfrever
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
___
Changes by Jesús Cea Avión j...@jcea.es:
--
nosy: +jcea
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
___
___
Python-bugs-list mailing list
Mark Dickinson added the comment:
BTW, there's also a behaviour change here. Before the patch:
True 0
1
After the patch:
True 0
True
Which demonstrates another good reason to avoid trivial-looking optimisations:
it's far too easy to accidentally change behaviour.
--
Mark Dickinson added the comment:
Can you sow the overhead of the branch in a microbenchmark?
Conversely, can you show a case where this optimisation provides a benefit in
real code? We should be looking for a reason *to* apply the patch, not a
reason *not* to apply the patch.
--
STINNER Victor added the comment:
The reason to apply the patch is to reduce the memory footprint.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
___
Antoine Pitrou added the comment:
Reduce the memory footprint in which actual workload? This looks rather
gratuitous to me.
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21422
New submission from STINNER Victor:
I propose to add a micro-optimization for int 0: return the number
unmodified.
See attached patch.
--
files: long_lshift0.patch
keywords: patch
messages: 217822
nosy: haypo, mark.dickinson, serhiy.storchaka
priority: normal
severity: normal
status:
Raymond Hettinger added the comment:
Every branch has a cost (in particular, it tends to contaminate global branch
prediction tables and blow other code out of the L1 code cache). The cost
isn't big, but branches shouldn't be added unless we know there is a real
benefit.
I would think that
STINNER Victor added the comment:
I would think that in real-world code, this branch will almost never be
taken. The common case will pay a price (albiet a small one) for almost
zero benefit.
I think that x 0 is common even if it's not written like that (it's more
for i in range(8): ... x
19 matches
Mail list logo
