Fix: Rpush magic doesn't find local variables and doesn't support comma separated lists of variables #2552

Closed
wants to merge 21 commits into
from

3 participants

@rmcgibbo

Described in issue #2550. This is a simple fix

@bfroehle bfroehle and 1 other commented on an outdated diff Nov 6, 2012
IPython/extensions/rmagic.py
@@ -199,10 +201,23 @@ def Rpush(self, line):
Out[11]: array([ 6.23333333])
'''
+ # split the line into fields separated by commas or spaces
+ # i.e. 'a,b', 'a b', 'a, b', 'a ,b', 'a , b' should all be split
+ # to ['a', 'b']
+ inputs = re.split('\s*[,\s]\s*', line)
@bfroehle
bfroehle added a line comment Nov 6, 2012

This is fine, or line.replace(',', ' ').split(), which is equally hard to read. The comment is quite helpful.

Also it'd probably be better to use a raw string, like:

inputs = re.split(r"...", line)
@rmcgibbo
rmcgibbo added a line comment Nov 6, 2012

done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bfroehle

Looks fine to me, but I haven't fired up R to test it.

@rmcgibbo

I should add some tests. There's already some tests in extensions/tests/test_rmagic.py, so I will just add to that.

@Carreau
IPython member

Hi, we don't get notified when smbd just push new commit,
So now there are test which is great.

Could smbdy that uses the R magic a little more than me have a look ?

@rmcgibbo

Thanks for the heads up @Carreau. I'll comment more vigorously next time.

@rmcgibbo

Damnit. I broke this branch by messing up at git.

I will close this PR, extract the code I actually want to submit, and try again.

@rmcgibbo rmcgibbo closed this Dec 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment