Skip to content

Conversation

@rajithv
Copy link
Contributor

@rajithv rajithv commented Jun 7, 2016

Defines the Abs class

c_one_arg_function = rb_define_class_under(m_symengine, "OneArgFunction", c_function);

//Abs Class
c_abs = rb_define_class_under(m_symengine, "Abs", c_function);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c_function -> c_one_arg_function

Btw, do we want one_arg_function? This was introduced in C++ to avoid code duplication, but I don't think there'll be any practical use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking it's better to keep the same class hierarchy as in C++, maybe it will be useful in the future. Currently there's absolutely no practical use though. What do you think? I can get rid of it if you want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add it later when an actual need arises.

It usually doesn't occur to anyone to remove something that is not being
used(as long as that doesn't obstruct anything) but it will occur to us if
we feel something is missing.

On Tue, Jun 7, 2016, 11:11 PM Rajith Vidanaarachchi <
notifications@github.com> wrote:

In ext/symengine/symengine.c
#50 (comment):

@@ -113,6 +113,10 @@ void Init_symengine() {
c_dirichlet_eta = rb_define_class_under(m_symengine, "Dirichlet_eta", c_function);
c_zeta = rb_define_class_under(m_symengine, "Zeta", c_function);
c_gamma = rb_define_class_under(m_symengine, "Gamma", c_function);

  • c_one_arg_function = rb_define_class_under(m_symengine, "OneArgFunction", c_function);
  • //Abs Class
  • c_abs = rb_define_class_under(m_symengine, "Abs", c_function);

I was thinking it's better to keep the same class hierarchy as in C++,
maybe it will be useful in the future. Currently there's absolutely no
practical use though. What do you think? I can get rid of it if you want.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/symengine/symengine.rb/pull/50/files/189089ac7e60c7ce7299f33f62fe63aa8d6bdb30#r66117718,
or mute the thread
https://github.com/notifications/unsubscribe/AGmQ6xP1b6DDIkMiCA8Cypitn0pGdp8oks5qJa0zgaJpZM4IwKla
.

@isuruf
Copy link
Member

isuruf commented Jun 7, 2016

Can you add some tests like abs(x+y) is a Abs

@isuruf
Copy link
Member

isuruf commented Jun 8, 2016

Looks good. See comment here.

@abinashmeher999
Copy link
Contributor

Looks good to me too.

@isuruf isuruf merged commit f23599d into symengine:master Jun 8, 2016
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

Successfully merging this pull request may close these issues.

3 participants