Adding -m option to %run, similar to -m for python interpreter. #819

Closed
wants to merge 5 commits into
from

Projects

None yet

3 participants

@jstenar
Member
jstenar commented Sep 22, 2011

I have long been missing an option to %run that is similar to -m for the regular python interpreter.

The -m option searches for a module on the python path and launches that module as a script.

This pull request is a first shot at this implementation. can you see anything that is missing?

I added some helper functions at top-level for locating modules, should they be somewhere else?

/Jörgen

@takluyver takluyver and 2 others commented on an outdated diff Sep 29, 2011
IPython/core/magic.py
mode='list',list_all=1)
+ if opts.has_key("m"):
+ modulename = opts.get("m")[0]
+ arg_lst = [find_mod(modulename)]
@takluyver
takluyver Sep 29, 2011 IPython member

This is currently going to replace all the arguments with the path to the module. Does Python itself allow you to do this sort of thing:

python -m somepkg.somemodule several args here

@fperez
fperez Oct 8, 2011 IPython member

@jstenar, one more thing for when you get a chance: let's use if "m" in opts instead of has_key, which is the more modern way to spell it out. We may have lingering uses of has_key but we shouldn't be using that for new code.

@fperez
fperez Oct 8, 2011 IPython member

And further, since you know that opts has m, then you can use modulename = opts['m'][0]. You only need to use get in dict access when you don't know for sure the key is available.

@jstenar
jstenar Oct 8, 2011 IPython member

Should I remove old has_key uses in magic_run as well?

@takluyver
takluyver Oct 8, 2011 IPython member

That's probably worth doing while you're working on it.

@jstenar
jstenar Oct 8, 2011 IPython member

I just pushed some changes that address these issues. I also did some pep-8 cleaning of magic_run.

@takluyver takluyver and 1 other commented on an outdated diff Sep 29, 2011
IPython/core/magic.py
+ """Get __init__ file path for module with directory dirname
+ """
+ fbase = os.path.join(dirname, "__init__")
+ for ext in [".py", ".pyw", ".pyc", ".pyo"]:
+ fname = fbase + ext
+ if os.path.isfile(fname):
+ return fname
+
+
+def find_mod(name):
+ """Find module *name* on sys.path
+ """
+ parts = name.split(".")
+ if len(parts) == 1:
+ basepath = find_module(parts[0])
+ else:
@takluyver
takluyver Sep 29, 2011 IPython member

Doesn't look like you actually need this if/else test. Just doing the bit in else should be enough.

@jstenar
jstenar Oct 8, 2011 IPython member

Changed in c16a5a0a

@takluyver
Member

Another quick question - if the user for some reason does %run -m (with no module name) or %run -m misspelt.modname, how gracefully will this fail?

@jstenar
Member
jstenar commented Sep 29, 2011

Thomas skrev 2011-09-29 22:16:

Another quick question - if the user for some reason does %run -m (with no module name) or %run -m misspelt.modname, how gracefully will this fail?

Thanks for your comments, you raise several good points I need to
adress. I guess that is why these reviews are a good idea :)

Unfortunatley I will not be able to look into this until late next week.

/Jörgen

@takluyver
Member

That's fine, there's no rush. I was just checking through the list of pull requests. Hope you're well!

@jstenar
Member
jstenar commented Oct 8, 2011

Running %run -m without argument will fail with an error during parse_options:

In [1]: %run -m
UsageError: option -m requires argument ( allowed: "nidtN:b:pD:l:rs:T:em:" )
@fperez
Member
fperez commented Oct 8, 2011

@jstenar, that error mode seems OK, as it gives the user enough information on what the issue is, so I'm OK with that. Did you see a problem with it?

On the other hand, due to other recent merges, this PR doesn't currently merge, so you'll need to give it a rebase and do a push --force. Let us know if you need a hand with any of that...

Jörgen Stena... added some commits Sep 7, 2011
Jörgen Stenarson Adding -m option to %run, similar to -m for python interpreter.
Added helper functions at top-level, should they be somewhere else?
55eff15
J�rgen Stenarson Improve error handling for %run -m 42da76c
J�rgen Stenarson Replace has_key with in in magic_run and some pep-8 fixes. 269e9c9
@jstenar
Member
jstenar commented Oct 9, 2011

I think I did the rebase correctly (my first one).

While I'm mucking about in magic_run I could take a stab at issue #710 as well or should I do that in a separate branch?

@takluyver
Member

I think let's do that in a separate branch - magic_run is some of the more thorny code, so it's worth keeping a clear eye on what's going on with it.

@jstenar
Member
jstenar commented Oct 9, 2011

Thomas skrev 2011-10-09 18:18:

I think let's do that in a separate branch - magic_run is some of the more thorny code, so it's worth keeping a clear eye on what's going on with it.

ok

@fperez fperez commented on an outdated diff Oct 9, 2011
IPython/core/magic.py
@@ -91,6 +91,49 @@ def needs_local_scope(func):
func.needs_local_scope = True
return func
+import imp, os
+
+def find_module(name, path=None):
+ """imp.find_module variant that only return path of module.
+ Return None if module is missing or does not have .py or .pyw extension
+ """
@fperez
fperez Oct 9, 2011 IPython member

Let's try to follow the dev guidelines for docstrings; here it is for reference:

http://ipython.org/ipython-doc/dev/development/doc_guide.html#docstring-format

This ensures that we'll have consistency throughout, and that auto-generated API docs have the right signatures.

We just use the numpy standard, so it's nothing particularly complicated or special-case.

@fperez fperez commented on an outdated diff Oct 9, 2011
IPython/core/magic.py
@@ -91,6 +91,49 @@ def needs_local_scope(func):
func.needs_local_scope = True
return func
+import imp, os
@fperez
fperez Oct 9, 2011 IPython member

In general, we keep imports at the top of the file and in their own line, alphabetically sorted. Just see the top of the file.

That makes it easier to see at a glance what any given module needs. The only exception is for particularly expensive or potentially conflicting dependencies, that get imported inside the functions that use them (such as gui tools).

@fperez
Member
fperez commented Oct 9, 2011

@jstenar, thanks for the updated work! I think we're almost there. But I see there are three new functions in the magic module now, so in addition to my in-line feedback, a couple of more general comments:

  • they are fairly generic utility functions related to module handling, so they should probably go elsewhere. I'd suggest a new module in IPython.utils, named something like modpaths.py. You can find a template with the necessary boilerplate at /docs/source/development/template.py.
  • Now that you have a nice new module with just three functions in it, the next thing is to ensure that you also add in utils/tests a file named test_modpaths.py that tests each one of these in isolation.

This way, the code will be easier to reuse elsewhere and will be validated. We want to move away from putting more and more tools into massive files like magic.py, that is already very unweildy.

So with a bit more iteration, we're close to having this solid for merge. Thanks for being patient, but that's how we ensure we build a more robust project moving forward!

@fperez fperez added a commit that closed this pull request Oct 13, 2011
Jörgen Stenarson Add -m option to %run to launch a module as a script.
The new -m option searches for a module on the python path and launches
that module as a script.  This is similar to Python's -m command-line
flag in behavior, but while staying inside IPython.

Conflicts:
	IPython/core/magic.py

Closes gh-819.
a4cfe5c
@fperez fperez closed this in a4cfe5c Oct 13, 2011
@jstenar
Member
jstenar commented Oct 18, 2011

Fernando Perez skrev 2011-10-09 22:00:

@jstenar, thanks for the updated work! I think we're almost there. But I see there are three new functions in the magic module now, so in addition to my in-line feedback, a couple of more general comments:

  • they are fairly generic utility functions related to module handling, so they should probably go elsewhere. I'd suggest a new module in IPython.utils, named something like modpaths.py. You can find a template with the necessary boilerplate at /docs/source/development/template.py.
  • Now that you have a nice new module with just three functions in it, the next thing is to ensure that you also add in utils/tests a file named test_modpaths.py that tests each one of these in isolation.

This way, the code will be easier to reuse elsewhere and will be validated. We want to move away from putting more and more tools into massive files like magic.py, that is already very unweildy.

So with a bit more iteration, we're close to having this solid for merge. Thanks for being patient, but that's how we ensure we build a more robust project moving forward!

I have pushed changes related to these comments

@fperez
Member
fperez commented Oct 18, 2011

@jstenar: yes, I'd seen that, and the PR was merged 4 days ago :) Good work!

@jstenar
Member
jstenar commented Oct 19, 2011

Fernando Perez skrev 2011-10-19 01:30:

@jstenar: yes, I'd seen that, and the PR was merged 4 days ago :) Good work!

cool, guess I missed that:)

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Jörgen Stenarson Add -m option to %run to launch a module as a script.
The new -m option searches for a module on the python path and launches
that module as a script.  This is similar to Python's -m command-line
flag in behavior, but while staying inside IPython.

Conflicts:
	IPython/core/magic.py

Closes gh-819.
a34330c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment