[PATCH] Parser for simple Mathematica expressions added (David Lawrence)
Kirill Smelkov
> # HG changeset patch
> # User Ondrej Certik <ond...@certik.cz>
> # Date 1198535464 -3600
> # Node ID 720d4489a8c1644b21fd2f0e79b0b122a9f82f52
> # Parent 49161c43403580303219bb16a4b29f9c35977dd7
> Parser for simple Mathematica expressions added (David Lawrence).
>
> This resulted as part of the GHOP:
>
> http://code.google.com/p/google-highly-open-participation-psf/issues/detail?id=307
Lawrence, Ondrej, thanks for the patch.
> diff -r 49161c434035 -r 720d4489a8c1 sympy/parsing/parser.py
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/sympy/parsing/parser.py Mon Dec 24 23:31:04 2007 +0100
> @@ -0,0 +1,53 @@
First of all, I'd suggest to put it in sympy.parsing.mathemtica.
Also, there should be sympy/parsing/__init__.py, or otherwise it is
impossible to import sympy.parsing.<anything>
> +from re import match
> +from sympy import sympify
> +
> +def mathematica (s):
> +
I have a question: why to use nested functions?
Maybe a class will, or just toplevel functions will do here?
> + def parse (s):
> + s = s.strip()
> +
> + #Function call
> + m = match(r"\A([\.\w\s]+)\[(.+)\]\Z", s)
> + if m:
> + return translateFunction(m.group(1)) + "(" + parse(m.group(2)) + ")"
> +
> + #Parenthesized implied multiplication
> + m = match(r"\((.+)\)\((.+)\)", s) #(a)(b)
> + if m:
> + return "(" + parse(m.group(1)) + ")*(" + parse(m.group(2)) + ")"
> +
> + #Parenthesized expression
> + m = match(r"\A\((.+)\)\Z", s)
> + if m:
> + return "(" + parse(m.group(1)) + ")"
> +
> + #Implied multiplication
> + m = match(r"\A(.*[\w\.])\((.+)\)\Z", s) #a(b)
> + if m:
> + return parse(m.group(1)) + "*(" + parse(m.group(2)) + ")"
> + m = match(r"\A\((.+)\)([\w\.].*)\Z", s) #(a)b
> + if m:
> + return "(" + parse(m.group(1)) + ")*" + parse(m.group(2))
> + m = match(r"\A([\d\.]+)([a-zA-Z].*)\Z", s) #2a
> + if m:
> + return parse(m.group(1)) + "*" + parse(m.group(2))
> +
> + #Infix operator
> + m = match(r"\A(.+)([\^\-\*/\+=+])(.+)\Z", s)
> + if m:
> + return parse(m.group(1)) + translateOperator(m.group(2)) + parse(m.group(3))
> +
> + return s
> +
This seems ok at first glance.
> + def translateFunction (s):
> + if s[0:3] is 'Arc':
> + s[0:3] = 'a'
This is wrong.
1. comparing str objects with is is not reliable, and
2. str objects do not support slice assignment
as a consequence of 1 & 2 e.g. Arccos is not translated to acos
> + return s.lower()
> +
> + def translateOperator (s):
> + dictionary = {'^':'**', '==':' is '}
> + if s in dictionary:
> + return dictionary[s]
> + return s
I'm not sure translating '==' to 'is' is a good idea, see comments in
the test.
> +
> + return sympify(parse(s))
> \ No newline at end of file
> diff -r 49161c434035 -r 720d4489a8c1 sympy/parsing/tests/test_parser.py
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/sympy/parsing/tests/test_parser.py Mon Dec 24 23:31:04 2007 +0100
> @@ -0,0 +1,11 @@
> +from sympy.parsing.parser import mathematica
> +from sympy import sympify
> +
> +def test_mathematica():
> + d = {'Sin[x]^2':'sin(x)**2', '2(x-1)':'2*(x-1)', '3y+8':'3*y+8',
> + 'Arcsin[2x+9(4-x)^2]/x':'arcsin(2*x+9*(4-x)**2)/x', 'x+y':'x+y',
> + '355/113':'355/113', '2.718281828':'2.718281828', 'Sin[12]':'sin(12)',
> + 'Exp[Log[4]]':'exp(log(4))', '(x+1)(x+3)':'(x+1)*(x+3)',
> + 'Cos[Arccos[3.6]]':'cos(arccos(3.6))'}
I'd suggest to reformat it, so each testcase is on it' own line.
Also, as said above Arcxxx should be translated to axxx.
Also, there is no test fo '==', and I'm not sure sympify will eat what
translateOperator will produce for it.
> + for e in d:
> + assert mathematica(e) == sympify(d[e])
Overall looks like a good start, thank.
--
Всего хорошего, Кирилл.
http://landau.phys.spbu.ru/~kirr/aiv/
Ondrej Certik
>
> On Mon, Dec 24, 2007 at 11:31:11PM +0100, Ondrej Certik wrote:
> > # HG changeset patch
> > # User Ondrej Certik <ond...@certik.cz>
> > # Date 1198535464 -3600
> > # Node ID 720d4489a8c1644b21fd2f0e79b0b122a9f82f52
> > # Parent 49161c43403580303219bb16a4b29f9c35977dd7
> > Parser for simple Mathematica expressions added (David Lawrence).
> >
> > This resulted as part of the GHOP:
> >
> > http://code.google.com/p/google-highly-open-participation-psf/issues/detail?id=307
>
> Lawrence, Ondrej, thanks for the patch.
Thanks Kirill for a review.
>
>
> > diff -r 49161c434035 -r 720d4489a8c1 sympy/parsing/parser.py
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/sympy/parsing/parser.py Mon Dec 24 23:31:04 2007 +0100
> > @@ -0,0 +1,53 @@
>
> First of all, I'd suggest to put it in sympy.parsing.mathemtica.
Agree.
> Also, there should be sympy/parsing/__init__.py, or otherwise it is
> impossible to import sympy.parsing.<anything>
This one is a bug in Mercurial. "hg vi" shows the empty __init__.py file,
hg di, hg out -p don't. I am going to report it.
David will respond for the rest.
Ondrej
Ondrej Certik
David
> I have a question: why to use nested functions?
> Maybe a class will, or just toplevel functions will do here?
de-nested the functions.
> > + def translateFunction (s): ...
> This is wrong.
Fixed.
> I'm not sure translating '==' to 'is' is a good idea, see comments in
> the test.
> I'd suggest to reformat it, so each testcase is on it' own line.
> Also, there is no test for '==', and I'm not sure sympify will eat what
> Overall looks like a good start, thank.
<http://code.google.com/p/google-highly-open-participation-psf/issues/
detail?id=307>
Thanks!
-David
Kirill Smelkov
> # HG changeset patch
> # User Ondrej Certik <ond...@certik.cz>
> # Node ID 821d57c9a50a1151ee0965cdb25d408d39b15b34
> # Parent 49161c43403580303219bb16a4b29f9c35977dd7
> Parser for simple Mathematica expressions added (David Lawrence).
>
> This resulted as part of the GHOP:
>
> http://code.google.com/p/google-highly-open-participation-psf/issues/detail?id=307
>
David, Ondrej,
> diff --git a/sympy/parsing/__init__.py b/sympy/parsing/__init__.py
> new file mode 100644
> diff --git a/sympy/parsing/mathematica.py b/sympy/parsing/mathematica.py
> new file mode 100644
> --- /dev/null
> +++ b/sympy/parsing/mathematica.py
> @@ -0,0 +1,52 @@
> +from re import match
> +from sympy import sympify
> +
> +def mathematica (s):
> + return sympify(parse(s))
> +
> +def parse (s):
> + s = s.strip()
> +
> + #Function call
> + m = match(r"\A(\w+)\[([^\]]+[^\[]*)\]\Z", s)
> + m = match(r"\A([^=]+)([\^\-\*/\+=]=?)(.+)\Z", s)
> + if m:
> + return parse(m.group(1)) + translateOperator(m.group(2)) + parse(m.group(3))
> +
> + return s
> +
This seems to be ok.
If you are intrested, I'd suggest to refactor parse() into set of rules,
so that the code will look like:
def parse():
for rule, handler in parse_rules:
m = match(rule)
if m:
return handler(m)
parse_rules = [
#Function call
(r"\A(\w+)\[([^\]]+[^\[]*)\]\Z",
lambda m: translateFunction(m.group(1)) + "(" + parse(m.group(2)) + ")"),
#Parenthesized implied multiplication
(r"\((.+)\)\((.+)\)", s) #(a)(b),
lambda m: "(" + parse(m.group(1)) + ")*(" + parse(m.group(2)) + ")"
...
]
But I'm ok if you'll leave it as is.
> +def translateFunction (s):
> + if s[0:3] == "Arc":
> + return "a" + s[3:]
> + return s.lower()
> +def translateOperator (s):
> + dictionary = {'^':'**'}
> + if s in dictionary:
> + return dictionary[s]
> + return s
If you have time, please add support for "/." substitution operator or
other Mathematica species. Again, I'm ok if you'll leave it as is.
> \ No newline at end of file
> diff --git a/sympy/parsing/tests/test_mathematica.py b/sympy/parsing/tests/test_mathematica.py
> new file mode 100644
> --- /dev/null
> +++ b/sympy/parsing/tests/test_mathematica.py
> @@ -0,0 +1,21 @@
> +from sympy.parsing.mathematica import mathematica
> +from sympy import sympify
> +
> +def test_mathematica():
> + d = {'Sin[x]^2':'sin(x)**2',
> + '2(x-1)':'2*(x-1)',
> + '3y+8':'3*y+8',
> + 'Arcsin[2x+9(4-x)^2]/x':'asin(2*x+9*(4-x)**2)/x',
What is the reason you are mixing tabs and spaces? Please try to avoid
it -- it is bad style and will break sooner or later.
Either use all-tabs, or all-spaces, but preferable stick to SymPy
convention (soft-tab = 4 spaces).
> + 'x+y':'x+y',
> + '355/113':'355/113',
> + '2.718281828':'2.718281828',
> + 'Sin[12]':'sin(12)',
> + 'Exp[Log[4]]':'exp(log(4))',
> + '(x+1)(x+3)':'(x+1)*(x+3)',
> + 'Cos[Arccos[3.6]]':'cos(acos(3.6))',
> + 'Cos[x]==Sin[y]':'cos(x)==sin(y)'}
> + for e in d:
> + try:
> + assert mathematica(e) == sympify(d[e])
> + except AssertionError:
> + print e
What if a test fails? You are just printing e, and all?
When run under py.test this will not indicate a failure.
--
Guys, it seems we are going to iter3, and I'm crossing my fingers, we'll
finally merge then :)
Ondrej Certik
Let's use spaces, tabs will cause problems when copying to/from other
sympy module.
Most python people and a standard library use spaces, so let's use what majority
of other people use - e.g. spaces.
>
> > + 'x+y':'x+y',
> > + '355/113':'355/113',
> > + '2.718281828':'2.718281828',
> > + 'Sin[12]':'sin(12)',
> > + 'Exp[Log[4]]':'exp(log(4))',
> > + '(x+1)(x+3)':'(x+1)*(x+3)',
> > + 'Cos[Arccos[3.6]]':'cos(acos(3.6))',
> > + 'Cos[x]==Sin[y]':'cos(x)==sin(y)'}
> > + for e in d:
> > + try:
> > + assert mathematica(e) == sympify(d[e])
> > + except AssertionError:
> > + print e
>
> What if a test fails? You are just printing e, and all?
>
> When run under py.test this will not indicate a failure.
I think it's because David is not yet used to py.test. Simply removing the
try: / except clauses will fix this problem.
> --
>
> Guys, it seems we are going to iter3, and I'm crossing my fingers, we'll
> finally merge then :)
As to extensions, let's see if David is interested. If not, let's just
commit this patch, it's ok as it is.
I can fix the remaining technical things (space + try/except) myself, that's
not a problem. So let's wait for David's reply, about his future plans
with this.
Ondrej
David
> If you are intrested, I'd suggest to refactor parse() into set of rules
> If you have time, please add support for "/." substitution operator or
> other Mathematica species. Again, I'm ok if you'll leave it as is.
reference.wolfram.com/mathematica/ref/ReplaceAll.html>. It seems
incredibly complicated to implement, so if it's all right with you,
I'd like to pass on this one.
> What is the reason you are mixing tabs and spaces? Please try to avoid
> it -- it is bad style and will break sooner or later.
> Either use all-tabs, or all-spaces, but preferable stick to SymPy
> convention (soft-tab = 4 spaces).
auto-indenter set to use tabs, but I was typing four spaces for each
indentation level.
This error has now been corrected.
> What if a test fails? You are just printing e, and all?
> When run under py.test this will not indicate a failure.
> Guys, it seems we are going to iter3, and I'm crossing my fingers, we'll
> finally merge then :)
really appreciate it.
The updated files have been posted at the GHOP issue page.
-David
Kirill Smelkov
> # HG changeset patch
> # User Ondrej Certik <ond...@certik.cz>
> # Node ID 54c7ceb12a7526d5ff35fa92145a6ee5e3492f25
> # Parent 49161c43403580303219bb16a4b29f9c35977dd7
> Parser for simple Mathematica expressions added (David Lawrence).
>
> This resulted as part of the GHOP:
>
> http://code.google.com/p/google-highly-open-participation-psf/issues/detail?id=307
>
> See also #161
Nice work, congratulations!