classification
Title: WeakSet should allow discard and remove on items that can't have weak references
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.8
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 2018-03-27 03:24 by rhettinger.

Pull Requests
URL Status Linked Edit
PR 1176 open donkopotamus, 2017-04-19 05:20
Messages (7)
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.
msg314495 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-03-27 03:24
> my original motivating case was objects that are hashable but not weakly referenceable.  

It is probably not good design to extend discard() to handle types that aren't handled by the other methods.  To me, it isn't at all surprising that the business of a WeakSet is to handle objects that are both hashable and weak referenceable.  Muddying those waters may do more harm than good.  My recommendation is to decline the feature request.

One other thought.  As the Python world starts to encourage type annotations and static type checking, one benefit we would hope for is code with cleaner matches between the caller and callee signatures.  Accordingly, we shouldn't encourage people to write code with odd signatures:
  
    x : thing_that_cant_possibly_be_in_the_weakset
    some_weakset.discard(x)  # Someday, we might hope this would be flagged
History
Date User Action Args
2018-03-27 03:24:05rhettingersetstatus: pending -> open
type: enhancement
messages: + msg314495

versions: + Python 3.8
2018-03-26 14:17:54serhiy.storchakasetstatus: open -> pending
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