[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: Methinks this one is also better rejected. Please reopen if you feel strongly. -- resolution: -> rejected status: open -> closed ___ Python tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Changes by Benjamin Peterson <[EMAIL PROTECTED]>: Added file: http://bugs.python.org/file10117/range_eq8_normalize5.patch __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Changes by Benjamin Peterson <[EMAIL PROTECTED]>: Added file: http://bugs.python.org/file10116/range_eq8_normalize4.patch __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: I merged your range calculating code. After reading that bug report, I think we need to start a discussion on python-dev about range size constraints before moving forward any more. (We have people implementing different things under differ

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: ... and your patch produces wrong results: >>> list(range(5,0,-2)) # expected [5, 3, 1] [5, 3] See my patch in issue2690 for a way to compute length correctly in range_new. __ Tracker <[EMAIL PROTECTED

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: With respect to range_eq8_normalize2.patch, it is unusual to have a function that consumes a reference to its argument. I would combine normalize_stop with PyNumber_Index and make it similar to validate_step with respect to reference

[issue2603] Make range __eq__ work

Changes by Benjamin Peterson <[EMAIL PROTECTED]>: Added file: http://bugs.python.org/file10113/range_eq8_normalize2.patch __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 7:10 PM, Guido van Rossum <[EMAIL PROTECTED]> wrote: .. > I think I'd be okay with normalization on creation, so that range(0, > 5, 2) returns range(0, 6, 2). Hm, but isn't that odd? Why not the > other way aro

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Here's a normalizing patch. It breaks the repr tests because the numbers change. Added file: http://bugs.python.org/file10111/range_eq8_normalize.patch __ Tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 3:41 PM, Benjamin Peterson <[EMAIL PROTECTED]> wrote: > If it makes any difference, the attributes aren't even available through > Python. But they are deducible via the str() or repr(). And IMO they *should* be av

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 5:38 PM, Guido van Rossum <[EMAIL PROTECTED]> wrote: > > Guido van Rossum <[EMAIL PROTECTED]> added the comment: > The two go hand-in-hand; you shouldn't have two range() objects that > compare equal and yet have

[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 3:36 PM, Benjamin Peterson <[EMAIL PROTECTED]> wrote: > > Benjamin Peterson <[EMAIL PROTECTED]> added the comment: > > Thanks for the help. > > Yes, after thinking for a while, I decided that range equality should

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Thanks for the help. Yes, after thinking for a while, I decided that range equality should represent the set of integers and not the values in the constructor. Normalization would be a good idea, but I think that's another issue I'll tackl

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 5:57 PM, Amaury Forgeot d'Arc <[EMAIL PROTECTED]> wrote: .. > - Did you change your mind about range equality? > range(0,10,2) == range(0,9,2) > seems True now; it was not with range_eq6.patch > This makes me

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: It looks like e-mail processor eats '>>>' examples. My examples were >>> range(2**100) == range(2**100+1) Traceback (most recent call last): File "", line 1, in OverflowError: Python int too large to convert to C ssize_t and >>> r

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Fri, Apr 25, 2008 at 5:31 PM, Benjamin Peterson <[EMAIL PROTECTED]> wrote: > Comments? In the range_hash function, len, start, step locals should be declared Py_ssize_t, not long. Also, you can use range_length() instead of PyObject

[issue2603] Make range __eq__ work

Amaury Forgeot d'Arc <[EMAIL PROTECTED]> added the comment: about range_eq7.patch: - Did you change your mind about range equality? range(0,10,2) == range(0,9,2) seems True now; it was not with range_eq6.patch - The hash function will fail with big values (and wrongly returns a value even whe

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Comments? __ Tracker <[EMAIL PROTECTED]> __ ___ Python-bugs-list mailing list Unsubscribe: http

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Attaching a new patch with a few improvements. I tried to implemented Raymond's hash function (I think this is how the math should be done.). The rich compare function now short-circuits when a value isn't equal. Also, I made ranges with th

[issue2603] Make range __eq__ work

Raymond Hettinger <[EMAIL PROTECTED]> added the comment: That was a typo. The pair was range(5,10,2)-->[5, 7, 9] and range (5,11,2)-->[5, 7, 9]. The formula works because the it uses seqlen instead of a stop value. __ Tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Amaury Forgeot d'Arc <[EMAIL PROTECTED]> added the comment: > It produces the same hash value for range(5,10,2) and range(5,9,2) > which are equivalent. If "equivalent" means "__eq__", they are not. This does not invalidate your formula, of course: different objects may have the same hash. It i

[issue2603] Make range __eq__ work

Raymond Hettinger <[EMAIL PROTECTED]> added the comment: No need to get hung-up on the hash function. I can fix that up after a checkin and use something simple like: hashvalue = (start*prime1+seqlen) *prime2+step. That doesn't involve object creation and it produces the same hash value for r

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Wed, Apr 16, 2008 at 9:24 PM, Benjamin Peterson wrote: .. > Why not just hash a tuple? There are a few reasons, but neither is good enough to have another round of code review :-) 1. It is strange to have the hash function allocate

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: When I was trying to get the hash to resemble tuple's, it occurred to me: Why not just hash a tuple? Added file: http://bugs.python.org/file10044/range_eq6.patch __ Tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Wed, Apr 16, 2008 at 3:40 PM, Antoine Pitrou <[EMAIL PROTECTED]> wrote: .. > Why would you want to have hash(range(a,b,c)) == hash((a,b,c)) ? No particular reason other than that this is easy to test and gives some assurance that ha

[issue2603] Make range __eq__ work

Antoine Pitrou <[EMAIL PROTECTED]> added the comment: Why would you want to have hash(range(a,b,c)) == hash((a,b,c)) ? It'd be more logical to have hash(range(a,b,c)) == hash(tuple(range(a,b,c))) ... which is not the same thing at all :) -- nosy: +pitrou

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: range_eq5.patch fails to reproduce tuple hash because in the tuple hash implementation the len variable varies between iterations over items. I would just use literals for the three different values of mult rather than copying mult += (l

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: With range_eq5.patch applied on an x86-64 Linux box: $ ./python Python 3.0a4+ (py3k:62359M, Apr 16 2008, 13:36:31) [GCC 3.4.6 20060404 (Red Hat 3.4.6-3)] on linux2 Type "help", "copyright", "credits" or "license" for more information.

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Thanks Alexander. Trying again. Added file: http://bugs.python.org/file10039/range_eq5.patch __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Tue, Apr 15, 2008 at 10:21 PM, Benjamin Peterson <[EMAIL PROTECTED]> wrote: > I'm not really sure what to do about the hash. I > just removed the Py_SIZE parts. I hope that's OK. If you want to maintain hash(range(a,b,c)) == hash((

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Attaching a new patch. I'm not really sure what to do about the hash. I just removed the Py_SIZE parts. I hope that's OK. Added file: http://bugs.python.org/file10038/range_eq4.patch __ Tracker <[EMAIL PROT

[issue2603] Make range __eq__ work

Amaury Forgeot d'Arc <[EMAIL PROTECTED]> added the comment: The patch uses Py_SIZE(v) on a rangeobject? I guess you borrowed too much from the tuple implementation ;-) __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: Close. Just a few nits: line 298: These two fit on one line. Possibly the whole thing fits. If it doesn't, the '{' should be on a line by itself. (Are you aware of PEP 7, C code guidelines? I realize it's incomplete, but you should still

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Thanks for the review! How does this look? Added file: http://bugs.python.org/file10037/range_eq_after_review.patch __ Tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: Code review: Objects/rangeobject.c: line 259-260: AFAIK register is completely useless in this day and age; drop it. line 296 and further: Please move the || and && operators to the end of the previous line. line 313 etc: This all fits o

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: The patch does not add unit tests for hash(range(..)). I would actually test that hash(range(a,b,c)) == hash((a,b,c)) for various values of a, b, and c. A nit-pick: while I personally like the coding style with line breaks before b

[issue2603] Make range __eq__ work

Guido van Rossum <[EMAIL PROTECTED]> added the comment: Once the review for this is completed I have no objection to it going in. -- nosy: +gvanrossum __ Tracker <[EMAIL PROTECTED]> __ ___

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Including __hash__ in this patch. Alexander, thanks; that could worked perfectly. Added file: http://bugs.python.org/file10001/range_eq3.patch __ Tracker <[EMAIL PROTECTED]>

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Thu, Apr 10, 2008 at 3:16 AM, Amaury Forgeot d'Arc <[EMAIL PROTECTED]> wrote: > > And, a __hash__ method should be added. > See issue2268 for a slice.__hash__ implementation which should work for range as well.

[issue2603] Make range __eq__ work

Amaury Forgeot d'Arc <[EMAIL PROTECTED]> added the comment: And, a __hash__ method should be added. -- nosy: +amaury.forgeotdarc __ Tracker <[EMAIL PROTECTED]> __ _

[issue2603] Make range __eq__ work

Benjamin Peterson <[EMAIL PROTECTED]> added the comment: Thanks for the help. Added file: http://bugs.python.org/file9995/range_eq2.patch __ Tracker <[EMAIL PROTECTED]> __

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: On Wed, Apr 9, 2008 at 6:08 PM, Georg Brandl <[EMAIL PROTECTED]> wrote: > Ah yes, and variable declarations must be at the start of a block You can avoid the need for extra local variables by declaring range_richcompare arguments as r

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: Actually, the patch contains an exploitable bug: $ cat x.py class X(int): def __eq__(self, other): raise ValueError x = range(X(1),X(10),X(1)) x == x $ ./python x.py Segmentation fault (core dumped) __

[issue2603] Make range __eq__ work

Georg Brandl <[EMAIL PROTECTED]> added the comment: Ah yes, and variable declarations must be at the start of a block, else MSVC (and possible other compilers) will complain. __ Tracker <[EMAIL PROTECTED]> ___

[issue2603] Make range __eq__ work

Georg Brandl <[EMAIL PROTECTED]> added the comment: I have no opinion whether the patch should go in, but in any case: * You should use PyObject_RichCompareBool. * You should error-check the returns of those. -- nosy: +georg.brandl __ Tracker <[EMAIL PRO

[issue2603] Make range __eq__ work

Alexander Belopolsky <[EMAIL PROTECTED]> added the comment: Your patch does not check the return values of PyObject_RichCompare calls for NULL. This is probably ok given the current restrictions on the type of start/step/stop, but adding the null checks will make the code more robust. -

[issue2603] Make range __eq__ work

New submission from Benjamin Peterson <[EMAIL PROTECTED]>: This patch makes this work: >>> range(4) == range(4) True This is in similar fashion to dict.keys >>> {1:2}.keys() == {1:2}.keys() True Tests included. -- components: Interpreter Core files: range_eq.patch keywords: patch messa