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

%load -s to load specific functions or classes #4311

Merged
merged 11 commits into from Oct 9, 2013

Conversation

mgaitan
Copy link
Contributor

@mgaitan mgaitan commented Sep 30, 2013

This PR adds the optional parameter -s to the magic %load that loads specific function or classes from the python source given. It's a complementary option to #4295.

A simple demo could be seen here http://nbviewer.ipython.org/6766501

… function or classes from the python source given. forcing the patch ignoring whitespace changes
@takluyver
Copy link
Member

Nice! I think there are cases where the end-of-block finding could go wrong, but they probably won't be a big problem in practice.

@Carreau
Copy link
Member

Carreau commented Sep 30, 2013

travis is angry though :

Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python2.7/doctest.py", line 1289, in __run
        compileflags, 1) in test.globs
      File "<doctest IPython.core.magics.code.extract_symbols[0]>", line 1
        extract_symbols(code, 'A,b'):
                                    ^
    SyntaxError: invalid syntax

code = code.split('\n')
for symbol in symbols.split(','):
if symbol in symbols_lines:
blocks.append('\n'.join(code[slice(*symbols_lines[symbol])]))
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be clearer broken up into a couple of lines. E.g.

start, end = symbols_lines[symbol]
blocks.append('\n'.join(code[start:end])

Also, a few lines of spacing and explanatory comments wouldn't go amiss throughout the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, simple is better than complex :)

@Carreau
Copy link
Member

Carreau commented Sep 30, 2013

Can some code be shared with ?? (%pinfo) which already extract code IIRC ?

@takluyver
Copy link
Member

That's finding the code for objects that are already loaded. We could run the module first, but that might have unwanted side effects.

However, it does use inspect.getblock() to find the end of blocks - that might be worth a look if we feel we need a better to find the end of a definition.

@damianavila
Copy link
Member

Good idea! I am agree with the comments adressed by @takluyver regarding spacing and explanatory comments.

@mgaitan
Copy link
Contributor Author

mgaitan commented Sep 30, 2013

On mgaitan/ipython@04432cc I've refactor the symbol parser function to get a better end. Previous version set that to the line were the next symbol begin - 1, but that could include many empty lines

I've also updated the demo to show this http://nbviewer.ipython.org/6766501

@Carreau
Copy link
Member

Carreau commented Oct 6, 2013

That looks really awesome.
I'm in favor in merging soon to get feedback.

@@ -239,6 +288,9 @@ def load(self, arg_s):

contents = self.shell.find_user_code(args)

if 's' in opts:
contents = '\n'.join(extract_symbols(contents, opts['s']))
Copy link
Member

Choose a reason for hiding this comment

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

There should be a message if any symbols are not found, maybe even an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


# construct a dictionary with elements
# {'symbol_name': (start_lineno, end_lineno), ...}
end = len(code)
Copy link
Member

Choose a reason for hiding this comment

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

Another quick comment on this line, please - something like "step backwards to a non-blank line". It just took me a minute to work it out.

@takluyver
Copy link
Member

This looks good. Apart from those couple of minor points, I'm in agreement with Matthias about merging this soon.

@ellisonbg
Copy link
Member

I too am strongly +1 on this one. Is it ready to go?

@mgaitan
Copy link
Contributor Author

mgaitan commented Oct 9, 2013

On Wed, Oct 9, 2013 at 5:05 PM, Brian E. Granger
notifications@github.comwrote:

Is it ready to go?

I think so :)

btw, thanks for all the feedback guys.

@mgaitan
Copy link
Contributor Author

mgaitan commented Oct 9, 2013

Just, one question . On mgaitan/ipython@f7cd9ed I've catch a bug introduced in mgaitan/ipython@7391345 . But that involves a design decision: When -s is used with non-python code, should warn "The symbols x,y... were not found" or something more specific like "Unable to parse as python ..." ?

in any case, if no symbol were found (because the input isn't python or the symbol/s is/are not present) should the empty cell be inserted anyway?

@ellisonbg
Copy link
Member

I think it should say something like "Unable to parse the file as Python..." and not insert the empty cell of the input file is not valid Python. Good catch.

@damianavila
Copy link
Member

I agree with @ellisonbg...

@mgaitan
Copy link
Contributor Author

mgaitan commented Oct 9, 2013

Ok, done.

@ellisonbg
Copy link
Member

Great, thanks.

ellisonbg added a commit that referenced this pull request Oct 9, 2013
%load -s to load specific functions or classes
@ellisonbg ellisonbg merged commit 446be78 into ipython:master Oct 9, 2013
@mgaitan mgaitan deleted the load_symbol_extractor branch October 9, 2013 21:10
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
%load -s to load specific functions or classes
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

6 participants