Wrap cse #193
Wrap cse #193
Conversation
| b = sympify(expr) | ||
| vec.push_back(b.thisptr) | ||
| symengine.cse(replacements, reduced_exprs, vec) | ||
| return (vec_pair_to_list(replacements), vec_basic_to_list(reduced_exprs)) |
There was a problem hiding this comment.
Are we sure on returning the results in a tuple? SymPy, if I'm not wrong, doesn't do that and might lead to an inconsistency later.
There was a problem hiding this comment.
SymPy does return a tuple. (At least in git master)
| @@ -81,7 +81,8 @@ install: | |||
|
|
|||
| - set PATH=C:\Python%PYTHON_VERSION%;C:\Python%PYTHON_VERSION%\Scripts;%PATH% | |||
| - pip install nose pytest | |||
| - pip install --install-option="--no-cython-compile" cython | |||
| - if [%COMPILER%]==[MinGW-w64] pip install --install-option="--no-cython-compile" cython==0.26 | |||
There was a problem hiding this comment.
@certik, you were right. Downgrading cython fixed the error
There was a problem hiding this comment.
Yeah. It's better to pick a particular version of Cython, since they seem to break things for us from time to time.
| @@ -81,7 +81,8 @@ install: | |||
|
|
|||
| - set PATH=C:\Python%PYTHON_VERSION%;C:\Python%PYTHON_VERSION%\Scripts;%PATH% | |||
| - pip install nose pytest | |||
| - pip install --install-option="--no-cython-compile" cython | |||
| - if [%COMPILER%]==[MinGW-w64] pip install --install-option="--no-cython-compile" cython==0.26 | |||
There was a problem hiding this comment.
Yeah. It's better to pick a particular version of Cython, since they seem to break things for us from time to time.
| e2 = (x + y)*w | ||
| substs, reduced = cse([e1, e2]) | ||
| assert substs == [(x0, x + y)] | ||
| assert reduced == [x0*z, x0*w] |
There was a problem hiding this comment.
I only added a few tests here to test the interface. All the tests in C++ are taken from the sympy test suite and there's no point reproducing here.
There was a problem hiding this comment.
Yes, it's just for the interface / API, not the correctness, which is (and must) be tested in C++.
|
Thanks for the reviews |
No description provided.