isi-vista / immutablecollections

A library for immutable collections, in the spirit of Guava's Immutable Collections.
MIT License
3 stars 2 forks source link

Make ImmutableSet support `-` operator when LHS is a regular set #13

Closed gabbard closed 5 years ago

gabbard commented 5 years ago

frozenset does this somehow.

gabbard commented 5 years ago

@qpwo : In case it is not clear, LHS here is "left-hand side"

qpwo commented 5 years ago

Unfortunately I don't think this cleanly possible. I think frozenset only can do it because that behavior is defined within set's code. Python always uses the left object's method for binary operators:

>>> class X:
...     def __sub__(self, other):
...         print("using X's method")
...
>>> class Y:
...     def __sub__(self, other):
...         print("using Y's method")
...
>>> x = X()
>>> y = Y()
>>> x - x
using X's method
>>> x - y
using X's method
>>> y - x
using Y's method
>>> y - y
using Y's method
>>>

It might be possible to override set's sub method to support ImmutableSet but that's against convention

gabbard commented 5 years ago

This issue was made long enough ago I don't remember much about it. @qpwo , what is the current behavior if you attempt to use - with a set LHC and a ImmutableSet RHS - I assume it is an exception?

qpwo commented 5 years ago

Yes a TypeError:


In [1]: import immutablecollections                                                                                                                                                             

In [2]: immutablecollections.immutableset([4,5,6])                                                                                                                                              
Out[2]: i{4, 5, 6}

In [3]: imset1 = immutablecollections.immutableset([4,5,6])                                                                                                                                     

In [4]: set1 = {2,3,4}                                                                                                                                                                          

In [5]: set1 - imset1                                                                                                                                                                           
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-b46689162942> in <module>
----> 1 set1 - imset1

/usr/local/miniconda3/lib/python3.7/_collections_abc.py in __rsub__(self, other)
    507                 return NotImplemented
    508             other = self._from_iterable(other)
--> 509         return self._from_iterable(value for value in other
    510                                    if value not in self)
    511 

/usr/local/miniconda3/lib/python3.7/_collections_abc.py in _from_iterable(cls, it)
    470         does not accept an iterable for an input.
    471         '''
--> 472         return cls(it)
    473 
    474     def __and__(self, other):

TypeError: __init__() missing 2 required positional arguments: 'iteration_order' and 'top_level_type'

In [6]: imset1 - set1                                                                                                                                                                           
Out[6]: i{5, 6}
berquist commented 5 years ago

I think you need to implement __rsub__ for ImmutableSets.

gabbard commented 5 years ago

@qpwo

Note If the right operand’s type is a subclass of the left operand’s type and that subclass provides the reflected method for the operation, this method will be called before the left operand’s non-reflected method. This behavior allows subclasses to override their ancestors’ operations.

suggests @berquist 's suggestion might work, depending on whether or not implementing AbstractSet is sufficient to count as "subclassing set"

qpwo commented 5 years ago

I got toy example to work so now I'll try to implement it in the main package