classification
Title: WeakSet should allow discard and remove on items that can't have weak references
Type: Stage:
Components: Library (Lib) Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: donkopotamus, fdrake, pitrou, rhettinger, serhiy.storchaka, stutzbach
Priority: normal Keywords:

Created on 2017-04-19 05:20 by donkopotamus, last changed 2017-04-24 04:25 by donkopotamus.

Pull Requests
URL Status Linked Edit
PR 1176 open donkopotamus, 2017-04-19 05:20
Messages (6)
msg291861 - (view) Author: (donkopotamus) * Date: 2017-04-19 05:20
Currently WeakSet().discard([]) will raise a TypeError as we cannot take a weak reference to a list.

However, that means a list can never be in a WeakSet, so WeakSet().discard([]) could instead be a no-op.  Similarly WeakSet().remove([]) could be a KeyError.
msg291867 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-19 08:30
What about difference_update(), issubset(), issuperset(), __eq__()?

Raising TypeError looks reasonable to me. Operations with ordinal sets can raise TypeError for unhashable values.

>>> [] in set()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'
>>> set().remove([])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'
>>> set().discard([])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'

Unlike to set.__contains__ WeakSet.__contains__ returns False for unsupported types:

>>> [] in weakref.WeakSet()
False

Shouldn't set.__contains__ be changed to return False for unhashable values? Or may be make WeakSet.__contains__ raising TypeError for values that can't have weak references?
msg291868 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-19 09:41
dict.pop() supports unhashable values, but dict.__contains__() and dict.get() don't:

>>> [] in {}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'
>>> {}.get([], 42)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'
>>> {}.pop([], 42)
42

OrderedDict.pop() also doesn't support unhashable values:

>>> collections.OrderedDict().pop([], 42)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'
msg291878 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-19 11:17
> Raising TypeError looks reasonable to me. 
> Operations with ordinal sets can raise TypeError 
> for unhashable values.

I agree.  TypeError looks the right exception in this case.
msg291880 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-19 11:48
The argument for not raising TypeError for unsupported types:

This is just from the definition of a set. If the set can't contain the value of specific type, then checking if this value is contained in the set should return False, and discarding it from the set should be no-op.

The argument for raising TypeError for unsupported types:

Using a value of unsuitable type is likely a programming error. Silencing TypeError can hide an error. Since raising and catching an exception is costly (but it is cheaper in C than in Python), this also can lead to performance degradation which is hard to catch.

If the first argument is stronger then the second argument then we should change a number of set, dict, dict views and OrderedDict methods and make them returning a reasonable result rather than raising TypeError.

If the second argument is stronger then the first argument then we should change just WeakSet.__contains__() and dict.pop(). Since this is a behavior change that can break third-party code they should first emit deprecation warnings for one or two releases.
msg291918 - (view) Author: (donkopotamus) * Date: 2017-04-19 22:18
Just to add, I framed this slightly wrongly by using a list [] as an example, whereas my original motivating case was objects that are hashable but not weakly referenceable.   (eg () ).

So there are two cases to consider ... what to do in discard/remove etc when the value is (i) not weakly referenceable, and (ii) when the value is not hashable.
History
Date User Action Args
2017-04-24 04:25:41donkopotamussettitle: WeakSet should all discard and remove on items that can have weak references -> WeakSet should allow discard and remove on items that can't have weak references
2017-04-19 22:18:59donkopotamussetmessages: + msg291918
2017-04-19 11:48:27serhiy.storchakasetmessages: + msg291880
2017-04-19 11:17:31rhettingersetmessages: + msg291878
2017-04-19 09:41:08serhiy.storchakasetmessages: + msg291868
2017-04-19 08:30:53serhiy.storchakasetnosy: + serhiy.storchaka, rhettinger, stutzbach
messages: + msg291867
components: + Library (Lib)
2017-04-19 05:54:29serhiy.storchakasetnosy: + fdrake, pitrou
2017-04-19 05:20:30donkopotamuscreate