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
Adding -m option to %run, similar to -m for python interpreter. #819
Conversation
mode='list',list_all=1) | ||
if opts.has_key("m"): | ||
modulename = opts.get("m")[0] | ||
arg_lst = [find_mod(modulename)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I remove old has_key uses in magic_run as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably worth doing while you're working on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed some changes that address these issues. I also did some pep-8 cleaning of magic_run.
Another quick question - if the user for some reason does |
Thomas skrev 2011-09-29 22:16:
Thanks for your comments, you raise several good points I need to Unfortunatley I will not be able to look into this until late next week. /Jörgen |
That's fine, there's no rush. I was just checking through the list of pull requests. Hope you're well! |
Running %run -m without argument will fail with an error during parse_options:
|
@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 |
Added helper functions at top-level, should they be somewhere else?
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? |
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. |
Thomas skrev 2011-10-09 18:18:
|
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 | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@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:
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! |
Fernando Perez skrev 2011-10-09 22:00:
|
@jstenar: yes, I'd seen that, and the PR was merged 4 days ago :) Good work! |
Fernando Perez skrev 2011-10-19 01:30:
|
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 ipythongh-819.
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