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.

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

Add REPL-like printing of final/return value to %%R cell magic #4017

Merged
merged 4 commits into from Aug 14, 2013

Conversation

gmbecker
Copy link
Contributor

This change makes the behavior of the %%R cell level magic in the rmagic extension agree with the behavior of both R code in other venues (the R program/RStudio/etc) and with python code within the notebook by treating the code/return value as if it was evaluated within a REPL.p

R's built-in facilities are used to both evaluate the code and determine whether the final value should be printed or not, guaranteeing correct behavior.

This change makes it much more convenient to use R within the notebook framework and also fixes the well-known issue of having to call print when using R's non-base graphics capabilities (ie lattice or ggplot2).

No change is made to the behaviour of the %R line-level magic.

repl_rmagic

the value generated by evaluating the code (see below)

In line mode (ie called with %R <stuff>), behavior
is unchanged, returns a python object representing
Copy link
Member

Choose a reason for hiding this comment

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

'unchanged' won't be meaningful in a year's time when people don't remember the previous behaviour.

@gmbecker
Copy link
Contributor Author

I have attempted to address the ambiguity and lack of clarity in the docstrings.

@@ -182,19 +182,42 @@ def __init__(self, shell, Rconverter=Rconverter,
self.pyconverter = pyconverter
self.Rconverter = Rconverter

def eval(self, line):
def eval(self, line, line_mode):
Copy link
Member

Choose a reason for hiding this comment

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

Let's give line_mode a default value so it's clear that it's a boolean.

@takluyver
Copy link
Member

On further thought, I think it would make more sense for the magic function to do the printing, so eval doesn't get any extra parameters, and returns a 3-tuple (text_output, value, visibility). There's already some logic around returning values which this should sit alongside.

Other than that, I think this is looking good.

@gmbecker
Copy link
Contributor Author

Good idea. I'll push the suggested changes to the PR shortly.

Thanks.

On Tue, Aug 13, 2013 at 3:23 PM, Thomas Kluyver notifications@github.comwrote:

On further thought, I think it would make more sense for the magic
function to do the printing, so eval doesn't get any extra parameters,
and returns a 3-tuple (text_output, value, visibility). There's already
some logic around returning values which this should sit alongside.

Other than that, I think this is looking good.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4017#issuecomment-22602400
.

Gabriel Becker
Graduate Student
Statistics Department
University of California, Davis

@ivanov
Copy link
Member

ivanov commented Aug 13, 2013

just want to link this to #3803 and make the PR author aware of the coming change.

text_output += text_result
if(visible):
Copy link
Member

Choose a reason for hiding this comment

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

if visible: doesn't need brackets in Python ;-)

@takluyver
Copy link
Member

Thanks, merging.

takluyver added a commit that referenced this pull request Aug 14, 2013
Add REPL-like printing of final/return value to %%R cell magic
@takluyver takluyver merged commit 681735e into ipython:master Aug 14, 2013
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Add REPL-like printing of final/return value to %%R cell magic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants