-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Rcode #11973
Rcode #11973
Conversation
fetched the latest updates
…t still translates into c not R
missing int piecewise expressions
tracing that will be erased later
also removed the verbose debugging printouts
with arrays (matrices)
tried to resolve conflicts, have to wait for the tests
…and doctests work
@@ -230,7 +230,8 @@ def order(self): | |||
|
|||
def doprint(self, expr): | |||
"""Returns printer's representation for expr (as a string)""" | |||
return self._str(self._print(expr)) | |||
res=self._str(self._print(expr)) | |||
return res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just a leftover of "print debugging..." I removed the print but forgot to inline it again, I will do it now along with the removal of the white space that I forgot as well..
# Unknown object, fall back to the emptyPrinter. | ||
return self.emptyPrinter(expr) | ||
res=self.emptyPrinter(expr) | ||
return res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any reason for this change either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above...
This is great! Thanks so much for this addition. A few things need to be fixed:
Other than, that I think this is looking great! |
Coverage report: 77% Show keyboard shortcuts |
Thanks for the nice reception and the work that comes with the review.
I tried to follow the hints.
Cheers
Markus
2016-12-23 21:46 GMT+01:00 Jason K. Moore <notifications@github.com>:
… This is great! Thanks so much for this addition.
A few things need to be fixed:
- Remove any trailing whitespace. You can run the test_code_quality.py
file to check. Also see the Travis failures.
- Can you add documentation? For example, make sure the API docs show
up like: http://docs.sympy.org/latest/modules/printing.html?
highlight=ccode#sympy.printing.ccode
<http://docs.sympy.org/latest/modules/printing.html?highlight=ccode#sympy.printing.ccode>.
And also would you mind adding a bit to here:
http://docs.sympy.org/latest/modules/codegen.html#code-
printers-sympy-printing
<http://docs.sympy.org/latest/modules/codegen.html#code-printers-sympy-printing>
- Can you run bin/coverage_report.py and report the results of your
test coverage here?
Other than, that I think this is looking great!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11973 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABETPTMKeohYgjVwykdXX8YVhe4hF9L8ks5rLDNBgaJpZM4LU6U5>
.
|
The key one is this: sympy/printing/rcode.py 151 49 1 68% Which is better than average for the code printers. That's fine for now I suppose, but it would be nice to increase the test coverage. |
|
||
return getattr(self, printmethod)(expr, *args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still needs to be fixed.
class RCodePrinter(CodePrinter): | ||
"""A printer to convert python expressions to strings of c code""" | ||
printmethod = "_rcode" | ||
language = "C" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines still have references to C.
I think that I have addressed all issues by now. |
I restarted the failed test on travis and it passes. |
Thanks a lot. |
I implemented an Rcode printer. I basically copied the ccode printer along with its tests and adapted both to produce valid R code from symy expressions. I do not know about any problems.
I have been using part of the functionality to check numerical solutions of an R package against symbolic solutions computed with sympy and then translated to R code automatically.