Vector multiplications
Triggered by discussion of #1344 and #1248
I'm more on the side of allowing __rmul__, since a lot of people have the habit of writing equations in such order. On implementation level it would be also tricky to show a warning to say that we don't allow it (in that case __rmul__ is anyway needed).
Also adding support for element-wise multiplication and division for vector * vector, aligning with other libraries like Numpy, Tensorflow, Pytorch, R ... (In MATLAB this returns dot product tho)
The PR is created more for discussion purpose, let me know what you guys think
What type of change is this?
- [ ] Bug fix in a backwards-compatible manner.
- [x] New feature in a backwards-compatible manner.
- [ ] Breaking change: bug fix or new feature that involve incompatible API changes.
- [ ] Other (e.g. doc update, configuration, etc)
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
- [x] I added a line to the
CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed). - [x] I ran all tests on my computer and it's all green (i.e.
invoke test). - [x] I ran lint on my computer and there are no errors (i.e.
invoke lint). - [ ] I added new functions/classes and made them available on a second-level import, e.g.
compas.datastructures.Mesh. - [x] I have added tests that prove my fix is effective or that my feature works.
- [ ] I have added necessary documentation (if appropriate)
I like this, and the implementation looks good to me. I'm not sure how to assess correctly the potential side-effects of this though.
I like this, and the implementation looks good to me. I'm not sure how to assess correctly the potential side-effects of this though.
I found this explanation for these functions:
In Python, the method __add__() and __radd__() are both part of the data model used to define how objects can be added together. The priority between these two methods depends on the operands involved in the addition operation. Here's how it works:
__add__() Method:
This method is attempted first when you use the + operator. It is a method for the left-hand operand of the addition.
For instance, in the expression a + b, Python first tries to call a.__add__(b).
__radd__() Method:
This method is the "right-side" addition method. It is called if the left operand's __add__() method does not support the addition with the right operand (either it returns NotImplemented, or the left operand doesn't have an __add__() method at all).
For instance, if a.__add__(b) returns NotImplemented, Python will check if b has a __radd__() method and if so, it will call b.__radd__(a).
In summary, __add__() has priority over __radd__() as it is attempted first. If __add__() fails to return a valid result and does not raise an exception, then __radd__() is attempted.
I will expand the test a bit here, to see what happens when different types are multiplied/added together
@tomvanmele @gonzalocasas What do you guys think about this way? If other is not a number, then we try casting the input into a Vector using Vector(*other) , this can allow more flexibility for other type of iterables
As for possible side effects of these reverse operator functions, because we don't return NotImplemented, this means that we will force the program to use our vector operator functions as long as our Vector class shows up at either side of operator. Which is not a necessarily a bad thing no?
@tomvanmele Could you take a look again and this one and see if it can be merged?