Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding NotImplemented to standard operators in Quaternion #26355

Open
Kadelka opened this issue Mar 13, 2024 · 9 comments
Open

Adding NotImplemented to standard operators in Quaternion #26355

Kadelka opened this issue Mar 13, 2024 · 9 comments

Comments

@Kadelka
Copy link

Kadelka commented Mar 13, 2024

I think it's possible to change Quaternion.__add__(self,other) in class sympy.Quaternion (and other operators like add) without changing the actual code of sympy.Quaternion in any essential way by introducing a Blacklist with
Blacklist = []
in class definion of Quaternion and then to replace f.i.

def __add__(self,other):
    return add.self(other)

by

 def __add__(self,other):
    if type(other) in Blacklist:
        return NotImplemented
    else:
        return self.add(other)

Similarly for the other operators. This would allow constructs like a+b with Quaternion a and b of a type unknown to Quaternion. With a construct like above actually a+b would be resolved as self.__radd__(a) with self = b. __radd__ then can handle internally a+b`. Without replacement the interpreter does not know what to do. See also my issue

@Rohanberiwal
Copy link

Rohanberiwal commented Mar 14, 2024

from sympy import Quaternion

class NotImplQuaternion:
    def __init__(self, operation):
        self.operation = operation
    
    def __str__(self):
        return f"One of the quaternions is not implemented for {self.operation}"

class MyQuaternion:
    Blacklist = []

    def __init__(self, a=0, b=0, c=0, d=0):
        try:
            self.quaternion = Quaternion(a, b, c, d)
        except Exception as e:
            print("An error occurred while initializing MyQuaternion:", e)

    def __add__(self, other):
        try:
            if type(other) in MyQuaternion.Blacklist:
                return NotImplemented
            elif isinstance(other, (int, float)):
                return NotImplQuaternion("addition")
            else:
                result = self.quaternion + other.quaternion
                return MyQuaternion(*result.args)
        except Exception as e:
            print("An error occurred during addition:", e)

    def __sub__(self, other):
        try:
            if type(other) in MyQuaternion.Blacklist:
                return NotImplemented
            elif isinstance(other, (int, float)):
                return NotImplQuaternion("subtraction")
            else:
                result = self.quaternion - other.quaternion
                return MyQuaternion(*result.args)
        except Exception as e:
            print("An error occurred during subtraction:", e)

    def __mul__(self, other):
        try:
            if type(other) in MyQuaternion.Blacklist:
                return NotImplemented
            elif isinstance(other, (int, float)):
                return NotImplQuaternion("multiplication")
            else:
                result = self.quaternion * other.quaternion
                return MyQuaternion(*result.args)
        except Exception as e:
            print("An error occurred during multiplication:", e)

    def __truediv__(self, other):
        try:
            if type(other) in MyQuaternion.Blacklist:
                return NotImplemented
            elif isinstance(other, (int, float)):
                return NotImplQuaternion("division")
            else:
                result = self.quaternion / other.quaternion
                return MyQuaternion(*result.args)
        except Exception as e:
            print("An error occurred during division:", e)

    def __str__(self):
        real, imag_i, imag_j, imag_k = self.quaternion.args
        return f"Result of operation: ({real} + {imag_i}*i + {imag_j}*j + {imag_k}*k)"

q1 = MyQuaternion(1, 2, 3, 4)
q2 = MyQuaternion(5, 6, 7, 8)
result_add = q1 + q2
print("Result of addition:", result_add)
result_sub = q1 - q2
print("Result of subtraction:", result_sub)
result_mul = q1 * q2
print("Result of multiplication:", result_mul)
result_div = q1 / q2
print("Result of division:", result_div)

print("Below are the test cases the returns Not implemented")
regular_number = 5
result_add = q1 + regular_number
print("Result of addition:", result_add)
result_sub = q1 - regular_number
print("Result of subtraction:", result_sub)
result_mul = q1 * regular_number
print("Result of multiplication:", result_mul)
result_div = q1 / regular_number
print("Result of division:", result_div)

@Rohanberiwal
Copy link

Hi , Greetings !
I've enhanced the code to handle cases where one of the operands isn't a quaternion. Now, if one operand is a regular number, the output is None / Not implemented , indicating the operation couldn't be performed . Let me know if this works and this was what we were suppose to fix ! I have attached the code above . Thank you

@Kadelka
Copy link
Author

Kadelka commented Mar 14, 2024

Hallo Rohanberiwall,
your code works as it is. Thank you. But the problem is that it doesn't change Quaternion, but introduces a new class MyQuaternion with the consequence, that in all of my code I have to change Quaternion with MyQuaternion. And I do not know if this really works. With my simple workaround I only have to define the Blacklist in octonion.py resp. sedenion.py, nothing more.

@Kadelka
Copy link
Author

Kadelka commented Mar 14, 2024

Hallo Rohanberiwall,
as an example here is my patch dieter.py

from sympy import Quaternion, core

def dummy_add(self,other):
    if type(other) in Quaternion.Blacklist:
        return NotImplemented
    else:
        return self.add(other)

Quaternion.__add__ = dummy_add

def dummy_sub(self,other):
    if type(other) in Quaternion.Blacklist:
        return NotImplemented
    else:
        return self.add(other*-1)

Quaternion.__sub__ = dummy_sub

def dummy_mul(self,other):
    if type(other) in Quaternion.Blacklist:
        return NotImplemented
    else:
        return self._generic_mul(self, core.sympify(other,strict=True))

Quaternion.__mul__ = dummy_mul

def dummy_truediv(self,other):
    if type(other) in Quaternion.Blacklist:
        return NotImplemented
    else:
        return self * core.sympify(other)**-1

Quaternion.__truediv__ = dummy_truediv__

and after the definition of class(Octonion) I add

Quaternion.Blacklist = [Octonion]
from dieter import *

This works as intended.

Greetings,
Dieter

@Kadelka
Copy link
Author

Kadelka commented Mar 14, 2024

I think the content of dieter.py can be realized in quaternion.py without changing the logic of Quaternion in any way. Nothing more has to be changed! In the same way other operators can be changed similarly if needed.

@Rohanberiwal
Copy link

Hi
I've carefully reviewed your feedback. I understand your preference for minimal changes to the existing Quaternion class. I'll work on implementing the suggested approach using the patch file (dieter.py) to modify the behavior directly. I'll ensure that the solution aligns with our project requirements and provides a seamless integration with the existing codebase. Please give me some suggesstion on if we can make more classes or we have to stick to the classes that are already written in the code .
Thanks

@Kadelka
Copy link
Author

Kadelka commented Mar 14, 2024

Hallo,
I've taken a look into the documentation of sympy's Quaternion. There is no other existing operator which needs adaption. radd etc. are treated by the calling program. All the better if you can integrate return NotImplemented without a Blacklist.
Thanks

@Rohanberiwal
Copy link

Prefect ! I will work on this and get back to you . Thanks !!

@Rohanberiwal
Copy link

Hi ! This is still open right ? I have made a few modification , I will be sharing it soon .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants