-
Notifications
You must be signed in to change notification settings - Fork 67
Sympy compat: add a functions submodule, add atan2 #118
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Left one comment.
cls = BasicMeta.__new__(mcls, name, bases, dict) | ||
if not name.startswith("_"): | ||
setattr(functions, name, cls) | ||
return cls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is name
attribute the main purpose of this? If so, you can add it to, https://github.com/symengine/symengine.py/blob/master/symengine/lib/symengine_wrapper.pyx#L1092-L1100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main purpose of this is to register all the special functions in this current file into the functions
submodule so that it behaves like sympy.functions (as an alternative, I could just go ahead and make a separate file for the module). I'm not 100% sure how your suggestion fits into this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, then that's fine. Can you move setting the name
attribute to symengine_wrapper.pyx
? Otherwise (sin(x) + 1) - 1
wouldn't have the name attribute, since it's only added if the function is directly created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont't think I understand what you mean, could you please explain a bit more? This code is not trying to add an attribute to the expression (sin(x) + 1) - 1)
. It's operating on the class sympy_compat.sin
when gets created as a class (metaclass registry pattern).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. It's operating on the class, not the object.
This PR is similar to #117 in that it intends to make things closer to sympy, but it contains changes to sympy-compat.
Summary:
cc: @inducer