Skip to content

Octave magics #1849

Merged
merged 12 commits into from Jun 13, 2012

5 participants

@stefanv
stefanv commented Jun 4, 2012

This PR adds %%octave, %octave, %octave_pull and %octave_push magics to facilitate interaction with Octave via oct2py.

Example notebook: https://gist.github.com/2866899

Example output: http://mentat.za.net/refer/octave_magic.pdf

@stefanv
stefanv commented Jun 4, 2012

Patch sent to oct2py author that fixes invalid capturing of + freezing on syntax errors.

@jenshnielsen

I have tested the magics and they seem to work nicely and are a great addition to the magics.

The tests should probably be excluded in iptest.py if oct2py is not found
similar to what is done for the R and Cython magics i.e.
jonathan-taylor@ee268de
in addition to the try except in the beginning of the test file.
This will allow the test report to display whether the ocs2py tests have been run.

@ellisonbg
IPython member

There is a couple of levels of test skipping that need to be done:

  • In the test file you need to set a flag to skip everything if oct2py is not found.
  • In iptest, you will have to exclude the extension and test file if oct2py is not found.

See the cython/R magics for how to do both of these things.

@fperez
IPython member
fperez commented Jun 10, 2012

Hey @stefanv, sorry but somehow this one picked up a conflict. Would you mind rebasing it? I'll be happy to finish the review so we can merge it asap.

@stefanv
stefanv commented Jun 10, 2012

@fperez Here we go!

@fperez
IPython member
fperez commented Jun 11, 2012

Test results for commit 353e51a merged into master
Platform: linux2

  • python2.7: OK

Not available for testing:

@fperez
IPython member
fperez commented Jun 11, 2012

@takluyver, any chance you could run test_pr on this one, after installing oct2py and h5py? For some reason on two different systems I can't get the py3 tests to even start running. I'm kind of baffled by what could be going on with test_pr there, but in the meantime it would also be good to know if this PR is OK on py3. The code looks solid and it's already had to go through rebase once, so I'd like to merge it before that happens again.

I'll try to dig into what's going on with py3 test_pr on my boxes, it's weird... It doesn't even report it as available, even though I certainly do have py3.

@fperez fperez commented on the diff Jun 11, 2012
IPython/extensions/octavemagic.py
@@ -0,0 +1,360 @@
+# -*- coding: utf-8 -*-
+"""
+===========
+octavemagic
+===========
+
+Magics for interacting with Octave via oct2py.
@fperez
IPython member
fperez added a note Jun 11, 2012

Since sometimes we do ship dependencies in externals, I think it's worth adding a word here telling the user that they need to install oct2py themselves for this to work (and mention that it in turn needs h5py, and doesn't declare the dependency right, so if you don't have h5py the easy_install fails).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez fperez commented on the diff Jun 11, 2012
IPython/extensions/octavemagic.py
+ [ 3., 4.]])
+
+ In [21]: y
+ Out[21]: 'hello'
+
+ '''
+ outputs = line.split(' ')
+ for output in outputs:
+ output = unicode_to_str(output)
+ self.shell.push({output: self._oct.get(output)})
+
+
+ @skip_doctest
+ @magic_arguments()
+ @argument(
+ '-i', '--input', action='append',
@fperez
IPython member
fperez added a note Jun 11, 2012

Note that with action append, it's possible to just call this multiple times. Users can pass -i x -i y -i z as well. I think you should either handle that case, or switch to a simple str type instead of an append type.

@stefanv
stefanv added a note Jun 11, 2012

Isn't this case already covered? I see it does a join and a split on ','?

@fperez
IPython member
fperez added a note Jun 11, 2012

Ah, right. I missed the ',' in the first join, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez fperez commented on the diff Jun 11, 2012
IPython/extensions/octavemagic.py
+
+
+ @skip_doctest
+ @magic_arguments()
+ @argument(
+ '-i', '--input', action='append',
+ help='Names of input variables to be pushed to Octave. Multiple names '
+ 'can be passed, separated by commas with no whitespace.'
+ )
+ @argument(
+ '-o', '--output', action='append',
+ help='Names of variables to be pulled from Octave after executing cell '
+ 'body. Multiple names can be passed, separated by commas with no '
+ 'whitespace.'
+ )
+ @argument(
@fperez
IPython member
fperez added a note Jun 11, 2012

This should be of type str with a store action, not append. Append is for lists only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez fperez commented on an outdated diff Jun 11, 2012
IPython/extensions/octavemagic.py
+ @argument(
+ '-i', '--input', action='append',
+ help='Names of input variables to be pushed to Octave. Multiple names '
+ 'can be passed, separated by commas with no whitespace.'
+ )
+ @argument(
+ '-o', '--output', action='append',
+ help='Names of variables to be pulled from Octave after executing cell '
+ 'body. Multiple names can be passed, separated by commas with no '
+ 'whitespace.'
+ )
+ @argument(
+ '-s', '--size', action='append',
+ help='Pixel size of plots, "width,height". Default is "-s 400,250".'
+ )
+ @argument(
@fperez
IPython member
fperez added a note Jun 11, 2012

Same here, this should be a string, not a list.

@fperez
IPython member
fperez added a note Jun 11, 2012

See the argparse docs for more details on how to use argparse...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez fperez commented on an outdated diff Jun 11, 2012
IPython/extensions/octavemagic.py
+
+ # Save output of the last execution
+ if exist("ans") == 1
+ _ = ans;
+ else
+ _ = nan;
+ end
+
+ for f = __ipy_figures
+ outfile = sprintf('%(plot_dir)s/__ipy_oct_fig_%%03d.png', f);
+ try
+ print(f, outfile, '-d%(plot_format)s', '-tight', '-S%(size)s');
+ end
+ end
+
+ ''' % {'plot_dir': plot_dir, 'size': size,
@fperez
IPython member
fperez added a note Jun 11, 2012

You can just do % locals() here since you're using all the same names and don't need to do sub-attribute access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez
IPython member
fperez commented Jun 11, 2012

OK, small and easy fixes to do. Once that's ready and we know if the suite passes on py3, this can go in. Great work, thanks!!

@stefanv
stefanv commented Jun 11, 2012

@fperez I incorporated your suggestions; please have a look.

@fperez
IPython member
fperez commented Jun 11, 2012

Looks great! One final request: please make a short notebook illustrating its use, sorry I forgot to mention that. You can see the R and Cython ones for inspiration; they make it carry much more punch and are the best way for new users to start with this.

@takluyver
IPython member

@fperez : w/ Py 3 testing, ensure that you have nose installed for Python 3 as well - it will report as unavailable if it can't import nose. Ubuntu precise has python3-nose in the repos.

@takluyver
IPython member

Unfortunately, it looks like oct2py isn't Python 3 compatible yet (though h5py is). It looks like it should be pretty easy to update, but I can't promise to get to it before 0.13.

I'm doing a test run anyway, just to ensure that the tests are correctly skipped without oct2py.

@takluyver
IPython member

Test results for commit 4b8ad1d merged into master
Platform: linux2

  • python2.7: Failed, log at https://gist.github.com/2909578 (libraries not available: oct2py pymongo wx wx.aui)
  • python3.2: OK (libraries not available: oct2py pymongo wx wx.aui)

Not available for testing: python2.6, python3.1

@takluyver
IPython member

That's a failure I see sporadically that's not related to this PR.

@fperez
IPython member
fperez commented Jun 11, 2012

Sounds good, thanks @takluyver for the report. I think this can go in once the example notebook is made, which should be very easy and quick to do. One more in for 0.13!

@stefanv
stefanv commented Jun 11, 2012

@fperez I made the notebook. I stripped all output before committing; do you prefer to have the figures inline?

@fperez
IPython member
fperez commented Jun 11, 2012

In this case I'd leave the figures in, so people who open it and don't have all the machinery at least can see what it looks like before installing. A few pngs in there aren't going to kill us.

@stefanv
stefanv commented Jun 11, 2012

@fperez Done

@fperez
IPython member
fperez commented Jun 11, 2012

Test results for commit 199aa71 merged into master
Platform: linux2

  • python2.7: OK (libraries not available: oct2py)
  • python3.2: OK (libraries not available: cython matplotlib oct2py pymongo rpy2 wx wx.aui)

Not available for testing:

@fperez
IPython member
fperez commented Jun 11, 2012

The notebook should read a bit like a document: at least it should have a main title (H1) and perhaps a couple of section headings for each magic along with a few words explaining. For example, the very top before loading the extension should mention the need for oct2py and h5py (crib from the docstring) so the user knows they need that before they just get a traceback, if they decide to try and run it.

@fperez
IPython member
fperez commented Jun 11, 2012

Otherwise I think it's great. So just add a few narrative paragraphs to make it read more like a document, and we're done. Thanks for your patience with our rather strict review process! I'm thrilled to see this go in.

@stefanv
stefanv commented Jun 13, 2012

I added some narrative to the notebook. It's still pretty sparse, so feel free to make further suggestions.

@fperez
IPython member
fperez commented Jun 13, 2012

Mmh, you forgot to push...

@stefanv
@fperez
IPython member
fperez commented Jun 13, 2012

Perfect, merging now. Thanks!!

@fperez fperez merged commit a8f8a66 into ipython:master Jun 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.