-
Notifications
You must be signed in to change notification settings - Fork 15
Add classes for Add, Mul and Pow #17
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
e = 2 * x | ||
assert e.to_s == '2*x' | ||
expect(e).to be_a SymEngine::Pow |
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.
Shouldn't this be SymEngine::Mul
?
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.
@abinashmeher999 fixed this
I think we should at least stick to |
@rajithv Just a suggestion. It is good to write the tests first. That way you are sure how you want your objects to behave in Ruby and not get influenced by the C like syntax. You can use |
@abinashmeher999 sure. |
Looks good to me. However, you will have to rebase once #15 is merged. The binary and unary operators were tested in |
@@ -157,7 +157,7 @@ | |||
z = SymEngine::Symbol.new('z') | |||
e = (x**y / z) | |||
f = e.free_symbols | |||
expect(f).to be_an_instance_of Set | |||
expect(f).to be_a Set |
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.
This change is unnecessary
+1 to merge |
+1 as well. Should we wait for #15 to be merged first? |
We should have the tests run on this once which would be after #15. |
Add classes for Add, Mul and Pow
@rajithv, thanks for the PR |
Changing the class of addition, multiplication and power expressions to
Add
,Mul
, andPow
to reflect SymEngine classes.