Add the __add__ function as __sub___ symetric
To keep the interface consitent: I use sub to remove a procset from an other now I can do the symetric call with add to increase a procset with an other
Merge request reports
Activity
Why not, I however have mixed feelings about this.
The main reason why there is no
__add__
as of today is to stick with the interface of standard containers. In the standard library, the__add__
and__or__
operations on iterable respectively are concatenation and union. The concatenation is meaningless with sets, while the union is properly defined.Last point, rather than creating a whole new
__add__
method, you can just write__add__ = __or__
in the definition of the class. (See the definition of__copy__
for an aliasing example). Also, as this is an alias of__or__
, I would declare it right after__or__
rather than next to the definition of__sub__
.Some minor points:
- Note that it is already implemented as
__or__
, you use it in your definition ;) - This may clash with the already existing
add
method: this however is not the best name. Now that you point this change, maybe the existingadd
would be better namedinsert
. This point is more about a possible rename than an argument against you proposal.
- Note that it is already implemented as
It's just an interface issue really: For me it it not intuitive to use
or
to do a union, If we need to use operators+
and-
seems more obvious. But I think using the plain text methods (like in interval_set) instead of operators is more explicit. The problem is that having several function to do the same thing goes against the python philosophy...Still, if I rewrite my merge request to define plain text methods (union, intersection, difference, ...) as alias for operator will you accept it
I agree about the add/insert comment, I think insert is more meaningful.
About the interface, I agree it is a personal matter ;)
Still, if I rewrite my merge request to define plain text methods (union, intersection, difference, ...) as alias for operator will you accept it
Actually, this is planned. Each dunder method for set operation is supposed to have a plain text method that support the operation with many sets (check docs/api.rst). These are not implemented yet, it'd be a useful contribution
I agree about the add/insert comment, I think insert is more meaningful.
I'll update the API accordingly in the next version.
mentioned in issue #7 (closed)