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.

Sign up for GitHub

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

Jump to bottom

Ruby wrapper for parser #60

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Ruby wrapper for parser #60

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
588fa76
Ruby wrapper for parser
rajithv Jul 8, 2016
cf77487
Update symengine version
rajithv Jul 8, 2016
300c769
testing parse method
rajithv Jul 8, 2016
5980a4a
Merge branch 'master' of https://github.com/symengine/symengine.rb in…
rajithv Jul 8, 2016
a2f88e2
Merge branch 'master' of https://github.com/symengine/symengine.rb in…
rajithv Aug 21, 2016
c20e7a0
Exception Handling and test with parse error
rajithv Aug 21, 2016
7e9ca53
Added more tests for parser
rajithv Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion ext/symengine/ruby_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,19 @@ VALUE cutils_sympify(VALUE self, VALUE operand)
sympify(operand, cbasic_operand);
result = Data_Wrap_Struct(Klass_of_Basic(cbasic_operand), NULL,
cbasic_free_heap, cbasic_operand);
return result;
}

VALUE cutils_parse(VALUE self, VALUE str)
{
VALUE result;
basic_struct *cbasic_result = basic_new_heap();

char *cstr = RSTRING_PTR(str);
basic_parse(cbasic_result, cstr);

result = Data_Wrap_Struct(Klass_of_Basic(cbasic_result), NULL,
cbasic_free_heap, cbasic_result);
return result;
}

Expand All @@ -26,6 +38,5 @@ VALUE cutils_evalf(VALUE self, VALUE operand, VALUE prec, VALUE real)
basic_evalf(cresult, cresult, NUM2INT(prec), (real == Qtrue ? 1 : 0));
result = Data_Wrap_Struct(Klass_of_Basic(cresult), NULL, cbasic_free_heap,
cresult);

return result;
}
4 changes: 4 additions & 0 deletions ext/symengine/ruby_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,9 @@

// Returns the Ruby Value after going through sympify
VALUE cutils_sympify(VALUE self, VALUE operand);
// Parses the expression in str and returns a SymEngine::Basic
VALUE cutils_parse(VALUE self, VALUE str);
// Evaluates the numerical value in operand (SymEngine::Basic) to precision
// defined in prec
VALUE cutils_evalf(VALUE self, VALUE operand, VALUE prec, VALUE real);
#endif // RUBY_UTILS_H_
3 changes: 3 additions & 0 deletions ext/symengine/symengine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ void Init_symengine()
rb_define_module_function(m_symengine, "convert", cutils_sympify, 1);
rb_define_global_function("SymEngine", cutils_sympify, 1);

// Parser as a Module Level Function
rb_define_module_function(m_symengine, "parse", cutils_parse, 1);

// evalf as a Module Level Function
rb_define_module_function(m_symengine, "_evalf", cutils_evalf, 3);

Expand Down
7 changes: 7 additions & 0 deletions spec/symengine_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,12 @@
it { is_expected.to be_a SymEngine::Rational }
its(:to_s) { is_expected.to eq '1/3' }
end

describe 'parse' do
subject { SymEngine::parse('123 + 321') }
Copy link
Member

Choose a reason for hiding this comment

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

Since these specs act as documentation, can you add more strings for parsing?
eg.

  • x + (y * 3 - 5)
  • x ** y + f(x)
  • sin(x) + cos(y) / 2
  • 1 / 2 + 3 / 2
  • 1 + 2 * 2
  • tan(0) - sqrt(2)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to add a few more to this list

  • 5+-3
  • 5--3

All Without spaces
And

  • 5++3 or something like this to raise an error.


it { is_expected.to be_a SymEngine::Integer }
it { is_expected.to eq 444 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will be if there's erroneous/unparseable input? (And, BTW, what input, if any is considered erroneous or unparseable by symengine?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zverok I did think about that. In C++ code it will throw errors for unparseable code. Checking that may almost be equivalent to doing the parsing. There are 4 errors,

  1. Expected token!
  2. Invalid symbol or number!
  3. Mismatching parantheses!
  4. Operator inconsistency!

Catching the exceptions from the C++ code and throwing ruby exceptions will be covered in the weeks 9 and 10. So this issue will be solved then.

As of now, I wanted the tests to merely confirm that the Ruby wrapper is communicating successfully with the cwrapper. Actually the same idea was behind the matrix wrapper tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. I just always became suspicious when I see tests for success and not tests for fail :)
In fact, I always prefer to add empty contexts for such a deferred tests pathes, like this:

context "When there is parsing error" do
  # TODO: will be covered on weeks 9-10
end

Though, I'm not insisting, currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a quite nice way to make it clear!

I'll keep this as it is, because week 9 is just one week away from now, probably I will start working on it even earlier.

end

end