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

The assumption in FunctionClass breaks subclassing UndefinedFunction with 3 args since SymPy 1.11 #26431

Open
utensil opened this issue Mar 31, 2024 · 2 comments
Labels

Comments

@utensil
Copy link

utensil commented Mar 31, 2024

In https://github.com/sympy/sympy/blob/master/sympy/core/function.py, the following lines in class FunctionClass

        # When __init__ is called from UndefinedFunction it is called with
        # just one arg but when it is called from subclassing Function it is
        # called with the usual (name, bases, namespace) type() signature.
        if len(args) == 3:
            namespace = args[2]
            if 'eval' in namespace and not isinstance(namespace['eval'], classmethod):
                raise TypeError("eval on Function subclasses should be a class method (defined with @classmethod)")

introduced by #23526 breaks the following minimal working example of subclassing UndefinedFunction with 3 args since SymPy 1.11:

from sympy import sympify, MatrixExpr
from sympy.core.function import AppliedUndef, UndefinedFunction

class MatrixFunction(UndefinedFunction):
    """ Like a MatrixSymbol, but for functions. """
    def __new__(mcl, name, m, n, **kwargs):
        cls = super().__new__(mcl, name, (AppliedUndef, MatrixExpr), {}, **kwargs)
        cls.shape = sympify(m, strict=True), sympify(n, strict=True)
        return cls

MatrixFunction('f', 3, 2)

The error is:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[4], line 1
----> 1 MatrixFunction('f', 3, 2)

File ....../sympy/core/function.py:194, in FunctionClass.__init__(cls, *args, **kwargs)
    192 if len(args) == 3:
    193     namespace = args[2]
--> 194     if 'eval' in namespace and not isinstance(namespace['eval'], classmethod):
    195         raise TypeError("eval on Function subclasses should be a class method (defined with @classmethod)")

TypeError: argument of type 'int' is not iterable

which will not error if one changes the args from name, m, n to e.g. name, shape, and is the indication the assumption is not robust for all cases.

See pygae/galgebra#495 for the original issue.

I wonder if a better way to trigger the validation can be found, or any more idiomatic suggestions for the use case are welcome.

@oscarbenjamin
Copy link
Contributor

We can just remove the validation check if it does not work correctly.

I am not sure though that subclassing UndefinedFunction is a good approach though. Overloading Function as both a superclass and a factory function was a design mistake. I separated them in gh-25103. Also the fact that UndefinedFunction uses dynamically generated classes is a design mistake.

I'm not sure what to suggest as a good way of doing this.

@utensil
Copy link
Author

utensil commented Apr 1, 2024

Thanks, it also seems to me that this legacy code is not doing it optimally. I've read https://docs.sympy.org/latest/guides/custom-functions.html and tried FunctionMatrix in SymPy, but it doesn't look obvious to me how to achieve the same result in a better way.

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

No branches or pull requests

2 participants