[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment: Fixed in r72751. -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: Why are you confused? You already asked that question earlier in the thread. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment: Maybe because I take the doc too specfic. It says A rich comparison method may return the singleton NotImplemented if it does not implement the operation for a given pair of arguments. I see the type check of the 'other' object as an operation towards the equal comparison, since it validates wether 'self' and 'other' can be equal at all. If they are of a different type, then they cannot be equal, thus the anwser to Are 'self' and 'other' equal? should be False. This again, would mean an equal operation is implemented and returning NotImplemented is not the right anwser. But going through similar code snippets shows otherwise, so my understanding must be lacking something. Therefore here is patch which returns NotImplemented. -- Added file: http://bugs.python.org/file13989/_weakrefset-notimplemented.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: I see the type check of the 'other' object as an operation towards the equal comparison, since it validates wether 'self' and 'other' can be equal at all. If they are of a different type, then they cannot be equal, thus the anwser to Are 'self' and 'other' equal? should be False. This again, would mean an equal operation is implemented and returning NotImplemented is not the right anwser. The explanation for NotImplemented is that an user-defined class may decide it can compare equal to a WeakSet (without inheriting from WeakSet). Returning NotImplemented from WeakSet.__eq__ gives a chance to the user-defined class' __eq__ method to be called. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: You can commit the latest patch, provided all tests pass. -- assignee: - schuppenies resolution: - accepted stage: patch review - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: Is the current patch ready for consumption? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment: The test passes on my machine, but a quick review would definitely be nice :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment: If that is the right behavior then yes. Is this documented somewhere? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Benjamin Peterson benja...@python.org added the comment: See http://docs.python.org/reference/datamodel.html#object.__eq__ -- nosy: +benjamin.peterson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Changes by Robert Schuppenies robert.schuppen...@gmail.com: -- stage: needs patch - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: 2. The current WeakSet implementation returns True if a WeakSet is compared to any Iterable which contains the same set of objects: Sounds bad. It should probably be fixed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment:
Sounds right to me. Here is another patch plus tests.
Going through the other tests, I adapted two more tests to actually test
WeakSet. Also, I found the following one and think it is a copypaste
from test_set which is not useful for test_weakset. Should it be removed
(as currently done in the patch)?
def test_set_literal(self):
s = set([1,2,3])
t = {1,2,3}
self.assertEqual(s, t)
--
Added file: http://bugs.python.org/file13957/_weakrefset.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5964
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Changes by Robert Schuppenies robert.schuppen...@gmail.com: Removed file: http://bugs.python.org/file13955/_weakrefset.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: Going through the other tests, I adapted two more tests to actually test WeakSet. Also, I found the following one and think it is a copypaste from test_set which is not useful for test_weakset. Should it be removed (as currently done in the patch)? Absolutely! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Antoine Pitrou pit...@free.fr added the comment: I don't think it is the intended behaviour. Comparison for equality should return either True or False (or perhaps NotImplemented?), not raise a TypeError. -- nosy: +pitrou priority: - normal stage: - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
Robert Schuppenies robert.schuppen...@gmail.com added the comment: Here is a patch which will return False instead of TypeError. This is the same behavior a normal set has. Two things though. 1. I don't know wether the 'import _abcoll' statement somehow influences the bootstrap in one way or the other, because 'from _abcoll import Iterable' does. 2. The current WeakSet implementation returns True if a WeakSet is compared to any Iterable which contains the same set of objects: import weakref class Foo: pass ... l = [Foo(), Foo(), Foo()] ws = weakref.WeakSet(l) ws == l True Not sure wether this is intended, since this is not the same behavior of a normal set. If it is intended, I think it should be documented. -- keywords: +needs review, patch Added file: http://bugs.python.org/file13955/_weakrefset.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5964] WeakSet cmp methods
New submission from Robert Schuppenies robert.schuppen...@gmail.com: Running this code: import weakref class C: pass ... ws = weakref.WeakSet([C]) if ws == 1: ... print(1) ... gives me the following exception: Traceback (most recent call last): File stdin, line 1, in module File /home/bob/python/svn/py3k/Lib/_weakrefset.py, line 121, in __eq__ return self.data == set(ref(item) for item in other) TypeError: 'int' object is not iterable Looking at _weakrefset.py line 121 gives def __eq__(self, other): return self.data == set(ref(item) for item in other) which treats any 'other' object as a set like object. Therefore comparing WeakSet to a non-set-like object always fails. Do I understand it correctly and if so, is this the intended behavior? -- components: Library (Lib) messages: 87420 nosy: schuppenies severity: normal status: open title: WeakSet cmp methods type: behavior versions: Python 3.0, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5964 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
