-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Add missing _print_Exp1
function to theanocode printer
#20335
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
Add `_print_Exp1` definition to the `theanocode` printing module. Printed functions containing a `sympy.core.numbers.Exp1` instance will fail to be printed. MWE to reproduce errors: ```python from sympy.printing.theanocode import TheanoPrinter import sympy x1 = sympy.Symbol('x1') TheanoPrinter()._print(sympy.exp(1)) ``` This gives the following traceback: ``` Traceback (most recent call last): File "/usr/lib/python3.8/code.py", line 90, in runcode exec(code, self.locals) File "<input>", line 1, in <module> File "/usr/lib/python3.8/site-packages/sympy/printing/printer.py", line 289, in _print return getattr(self, printmethod)(expr, **kwargs) File "/url/lib/python3.8/site-packages/sympy/printing/theanocode.py", line 156, in _print_Basic op = mapping[type(expr)] KeyError: <class 'sympy.core.numbers.Exp1'> ```
✅ Hi, I am the SymPy bot (v161). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.10. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
Codecov Report
@@ Coverage Diff @@
## master #20335 +/- ##
=============================================
- Coverage 75.748% 75.733% -0.015%
=============================================
Files 673 673
Lines 174050 174393 +343
Branches 41099 41198 +99
=============================================
+ Hits 131840 132074 +234
- Misses 36493 36601 +108
- Partials 5717 5718 +1 |
Can you add a test that this is working correctly? |
Otherwise looks good |
Just pushed a test case, let me know if it looks good. I noticed in the |
No idea. I don't know this part of the codebase so well. You could check the blame and see where the code was added. There might be some discussion there. Would it not make sense to test the output of the printer rather than (or as well as) the output from theano? |
Got you - I will explore the blame. I do believe the test tries out the printer - the def theano_code(expr, cache=None, **kwargs):
...
return TheanoPrinter(cache=cache, settings={}).doprint(expr, **kwargs) I didn't directly call |
Just following up on this. Following the other test cases and per my last response, this should sufficiently test the printer for Exp1 |
Closing and reopening to trigger a new test run. Seems OK to me. |
Looks OK to me! Enabling auto-merge so once the tests pass it will be merged. Thanks for your contribution! |
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) Full benchmark results can be found as artifacts in GitHub Actions |
References to other Issues or PRs
None, afaik
Brief description of what is fixed or changed
Add
_print_Exp1
definition to thetheanocode
printing module. Printed functions containing asympy.core.numbers.Exp1
instance currently fail to be printed.Other comments
MWE to reproduce errors:
This gives the following traceback:
Release Notes
TheanoPrinter
fails to print Euler's number e (sympy.core.numbers.Exp1
)