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

Use fire without editing code directly #29

Closed
jtratner opened this issue Mar 22, 2017 · 27 comments
Closed

Use fire without editing code directly #29

jtratner opened this issue Mar 22, 2017 · 27 comments

Comments

@jtratner
Copy link
Contributor

Currently you have to add something like this to the end of your file just to mess around with fire:

if __name__ == '__main__':
    fire.Fire()

and then run with

python path/to/code.py

But it'd be neat if instead you could just do

fire path/to/code.py

and that does equivalent of exec appending the ifmain statement to the end.

Not sure how to do it - I know kernprof does some magic to inject into builtins before exec'ing code. (or maybe you get a code object back from exit) - but this would make fire even better as a debugging tool as well.

@dbieber
Copy link
Member

dbieber commented Mar 22, 2017

Cool idea.

Let's think through some of the behavior specifics.

  • What happens if the file already has a main()? What if it's already using Fire or some other CLI library? Probably we don't want __name__ == '__main__' to be True when we run code.py, but inevitably if code.py has top-level code then it will be executed.
  • Python supports 'python -m' for running a module. Do we want to support 'fire -m' as well?
  • With regular Fire CLIs, we need to separate flags from the fire command with an isolated '--'. Here we could potentially allow the fire flags to preceed the filename. e.g. fire -i code.py or fire -h code.py command. Is this something we'd want?
  • I have a design doc coming for updating tab completion; we'll have to think about how this would play alongside tab completion.

@jtratner
Copy link
Contributor Author

Great point on executing ifmain by accident. This set me on to a neat idea - what if you could just pass in any fully qualified python module/function/class?

I snagged the AppEngine mapreduce's for_name to do the resolution and voila:

›› python fire/cli.py os.path basename a/b/c
c
›› python fire/cli.py os.path isfile a/b/c                                                                                                              
False
›› python fire/cli.py os.path exists a/b/c                                                                                                              
False
›› python fire/cli.py os.path exists fire/cli.py                                                                                                        
True

branch here with initial doodle - https://github.com/jtratner/python-fire/tree/run-on-any-fully-qualified-name

If we could get it to work so that --interactive ended up in the namespace of the module (rather than caller namespace) that'd make this incredibly useful.

I'll think about your other questions and get back to you later (have to get a deliverable together).

@jtratner
Copy link
Contributor Author

jtratner commented Mar 22, 2017

›› python fire/cli.py requests.get https://www.google.com - headers items
["Date", "Wed, 22 Mar 2017 21:29:50 GMT"]
["Expires", "-1"]
["Cache-Control", "private, max-age=0"]
["Content-Type", "text/html; charset=ISO-8859-1"]
["P3P", "CP=\"This is not a P3P policy! See https://www.google.com/support/accounts/answer/151657?hl=en for more info.\""]
["Content-Encoding", "gzip"]
["Server", "gws"]
["X-XSS-Protection", "1; mode=block"]
["X-Frame-Options", "SAMEORIGIN"]
[...]
["Transfer-Encoding", "chunked"]

@dbieber
Copy link
Member

dbieber commented Mar 23, 2017

That's nice 👍 .

I think we'd want to support 'fire filepath.py' as well, as in your original idea.

If we could get it to work so that --interactive ended up in the namespace of the module (rather than caller namespace) that'd make this incredibly useful.

Interesting. So, this would behave as if 'fire.Fire()' were appended to the module you're running fire against, as in your original description, yes?

Haven't tried this fully yet, but I think something like this could work:

import fire
import imp
module = imp.load_source('module', '/path/to/file.py')
from module import *
del imp
fire.Fire(module)  # or fire.Fire()?

@dbieber
Copy link
Member

dbieber commented Mar 23, 2017

Here's a working first pass for supporting file paths:

import fire

import imp
import sys
module = imp.load_source('module', sys.argv[1])
from module import *

if __name__ == '__main__':
  fire.Fire(module, command=' '.join(sys.argv[2:]))  # or fire.Fire()?

We'll want to replace 'command' with 'argv'.

@jtratner
Copy link
Contributor Author

The issue of __main__ namespace getting mangled is pretty big, e.g., it makes pickling really annoying and sometimes causes weird issues with imports.

Just to clarify what I mean about namespaces:

fire -m pandas -- --interactive
IPython shell!
$ read_csv('whatever')
$ import pandas as pd
$ assert read_csv is pd.read_csv 
$ assert DataFrame.__module__ != '__main__'

Under the hood would be essentially the same as what you have, but using import_module instead.

import importlib
module = importlib.import_module(name)
fire.Fire(module)

Which would then cause all of the module's locals (or maybe __dict__ to be in namespace).

So then you could end up with something like:

fire -m numpy genfromtxt 'myfile.csv' --delimiter=',' - sum

@jtratner
Copy link
Contributor Author

Python supports 'python -m' for running a module. Do we want to support 'fire -m' as well?

Could be a good way to simplify loading, but I'm not really sure what you'd gain there / what it would mean. (would that mean you run the module's main function?) Ideally I'd like fire to always import modules and classes with namespaces intact.

With regular Fire CLIs, we need to separate flags from the fire command with an isolated '--'. Here we could potentially allow the fire flags to precede the filename. e.g. fire -i code.py or fire -h code.py command. Is this something we'd want?

That would simplify the user experience a little - easy to understand "fire options come before specifying what to operate on".

I have a design doc coming for updating tab completion; we'll have to think about how this would play alongside tab completion.

Good question - seems like it should be able to work the same way. It would be cool to be able to "teach" fire how to handle nested completion, e.g. I say:

fire some_module <method1> <args> - <method 2> - <method 3>

and the completion script somehow dumps completions for methods1 2 and 3 ;)

@jtratner
Copy link
Contributor Author

Interesting. So, this would behave as if 'fire.Fire()' were appended to the module you're running fire against, as in your original description, yes?

Agreed. Exactly how I'd expect it to work. (and we could either automagically determine what user meant on CLI or have different flags depending on what you are referencing)

@jtratner
Copy link
Contributor Author

How do you feel about this as precedence for loading modules vs. loading classes / methods vs. loading files (pandas vs. pandas.read_csv vs. pandas.DataFrame)?

  1. check for a module, if True pass to fire.Fire and stop
  2. check if it's a class or method, if True pass to fire.Fire and stop
  3. assume it's a file, if exists, try to load with load_source
  4. error out

@dbieber
Copy link
Member

dbieber commented Mar 24, 2017

... and the completion script somehow dumps completions for methods1 2 and 3 ;)

Yes, would love to get to that point. I put a draft of the tab completion doc at #32.

Need to think more about the precedence idea. On the one hand, it's nice if a tool like this 'just works'. On the other hand, it's good to conform to standards set by tools like python and ipython. If we do go the precedence route, I suspect that we'd want file paths to take precedence over module names, but that's a detail.

@nealmcb
Copy link
Contributor

nealmcb commented Mar 26, 2017

Fire shares similarities with hug: Embrace the APIs of the future, which also generates RESTful web interfaces. Hug supports both a -m module option and a -f file option for starting up a web server, and a command-line option via the __main__ trick. It's probably worthwhile digging in to the implications of how they did it all as you design this feature in fire, which certainly sounds like a great idea to me.

Exploring how it works with some existing python modules is probably a good idea. E.g. I was delighted to to see @dbieber's quick implementation work like this:

$ cd /usr/lib/python3.4 
$ dbieberfire random.py
Type:        module
String form: <module 'module' from 'random.py'>
File:        /usr/lib/python3.4/random.py
Line:        1
Docstring:   Random variable generators.
.... more documentation....

$ dbieberfire random.py Random
....yet more specific documentation...

$ dbieberfire random.py Random gauss 50 5
53.698054343119814
$ dbieberfire calendar.py month 1957 6
     June 1957
Mo Tu We Th Fr Sa Su
                1  2
 3  4  5  6  7  8  9
10 11 12 13 14 15 16
17 18 19 20 21 22 23
24 25 26 27 28 29 30

But trying to use it for b85encode ran into the question of how to create a "bytes" input parameter

$ dbieberfire base64.py b85encode xyzzy
...
TypeError: memoryview: str object does not have the buffer interface

@nealmcb
Copy link
Contributor

nealmcb commented Mar 26, 2017

OK, I figured out that the join operation in @dbieber's version is what was breaking the use of arguments of type bytes, by turning them back into strings.

The attached python script (with a .txt prefix to keep github happy, but you should rename it fire) will load files (with -f option) and modules (with -m option), and leave bytes strings alone.

fire.txt

So now this works:

$ fire -m base64 b85encode "b'Hello world'" - decode                                                                 
NM&qnZy<MXa%^M
$ fire -m base64 b85decode "b'NM&qnZy<MXa%^M'" - decode                                                              
Hello world

nealmcb added a commit to nealmcb/python-fire that referenced this issue Mar 26, 2017
Based on the discussion at [Use fire without editing code directly · Issue google#29 · google/python-fire](google#29)

Of course, still needs better error handling, documentation, installation support, etc.
@dbieber
Copy link
Member

dbieber commented Mar 27, 2017

Glad you found the bytes workaround 👍 .

For sharing code like fire.txt, consider using a gist or pastebin in the future instead of attaching a text file.

Python question: is there a way to do the equivalent of "from X import *" inside a Python function? I know the naive approach gives a syntax error, since "import *" is only allowed at the module scope.

Design thought: I think we'll want a sensible default so that people don't have to specify -m or -f. I'm inclined to take the precedence approach (first check if it's a file path that exists. if not, check if it describes a Python object using the for_name approach that jtratner described.) Will have to think more carefully about ways in which this might violate the user's expectations though.

On the subject of Hug, one of my low-priority plans for Fire is to experiment with adding a --serve flag to Fire CLIs. Much like how Fire CLIs expose an object or a whole program as a CLI, a Fire Server would expose an object or a whole program over HTTP. I can open a separate issue if you want to discuss this further or are interested in exploring the idea / contributing.

@jtratner
Copy link
Contributor Author

jtratner commented Mar 27, 2017 via email

@nealmcb
Copy link
Contributor

nealmcb commented Mar 27, 2017

My general sense is that the PEP 20 advice Explicit is better than implicit applies here, and would lead to the explicit specification of whether the user wants a module or a file (e.g. via -f / -m), and not magically inferring that from an argument. That would make it harder to break a command by just running it in a different directory, for example. And I'm wondering if the for_name syntax and rules are necessary given the existing fire syntax and rules. But these are all good questions!

@jtratner
Copy link
Contributor Author

It seems very rare for a module or file to overlap (i.e., very uncommon to have modules whose name ends with .py) so I'd rather leave out the CLI arg if we don't need it (+ it's a really obvious change later if we start to require it).

@nealmcb
Copy link
Contributor

nealmcb commented Mar 28, 2017

@jtratner I think I'm warming to the idea of having a little magic here, since a reasonable goal for fire is probably minimizing the need for keystrokes. Hug does it the way I used (-m and -f), but I agree that the typical presence of a .py extension on files will help people avoid confusion in most cases.

And thanks for pointing out the autoreload aspects of the implementation. I'll have to look at that some more. I also wonder if having a __main__.py file in fire will cause problems, since that's what I just renamed fire.py to. Hmmm.

Lots of tricky bits here. A good set of test cases will be critical, so please keep them coming. The more concrete the examples, the better.

@dbieber
Copy link
Member

dbieber commented Mar 28, 2017

I wonder why hug uses both -m and -f, rather than -m for module and no flag for file (which is how python/ipython/pyspark do it). I'm not really familiar with hug, so I don't know if there's a compelling reason or if it was just an arbitrary choice (presumably motivated by explicit>implicit).

dbieber pushed a commit that referenced this issue Apr 5, 2017
* fire.py: Use fire on arbitrary file or module.

Based on the discussion at [Use fire without editing code directly · Issue #29 · google/python-fire](#29)

Of course, still needs better error handling, documentation, installation support, etc.

* rename fire.py to fire_script.py in repo

Should fix the build problem for python 2.  Later, get it installed as "fire" via setup.py

* rename fire_script.py __main__.py
Following advice from Python Apps the Right Way: entry points and scripts | Chris Warrick
 https://chriswarrick.com/blog/2014/09/15/python-apps-the-right-way-entry_points-and-scripts/
@nealmcb
Copy link
Contributor

nealmcb commented Apr 5, 2017

I think using fire with modules will be the most common usage, so I would lean toward @jtratner's notion that not requiring a -m option for it is the most helpful simplification. I guess it's ok to also not require a -f option, and also try to interpret the first argument at a file. But while Python files do nearly always have a .py extension, directory names can be loaded the same way, interpreted as modules (and often named the same). So we do have to be careful about which to make the default. I guess it's more obvious to the user how to give a fully-qualified file name to make it obvious that they want a file loaded, so my first notion is:

  1. try to load it as a module name via imp.find_module
  2. try to load module as a file or directory name via imp.load_source
  3. error out

I've left out trying to interpret the first argument as a class or method, since it seems more complex to specify syntax for those names than to use the existing fire approach to accessing those. But I'm open to examples that motivate adding that.

I've made a first shot at implementing the above in https://github.com/nealmcb/python-fire/tree/issue29-fire-without-edits
and the main issue I see is that error handling may be confusing, since we're not exactly sure what the user intended.

I'm happy to either add functionality as we go along via pull requests to the google branch, or playing for a while in my own repo.

@nealmcb
Copy link
Contributor

nealmcb commented Apr 7, 2017

Thanks, @jack17529, for offering to help with testing, at pull #35 (comment). I'm not sure, but I guess its better to continue the discussion here than in that old pull request.

There are a few aspects of testing the CLI.

First is how to implement the tests. We want it to fit in with and leverage the existing fire unittest approach, but also need to test the invocation of the command itself, probably including the script which setup.py will build. Input on exactly how to do that is most welcome.

Then of course we want some specific test cases to exercise the functionality, and cover some of the tricky aspects of this, like the __main__ namespace issue.

@dbieber
Copy link
Member

dbieber commented Apr 8, 2017

API

I think using fire with modules will be the most common usage

I actually suspect (just a guess of course!) that using it with files will be the most common usage. One reason for this is that everyone's default tab-completion completes files but not modules. Another reason is that I don't know of any command line tools that accept module specifications w/o a flag (e.g. python -m, ipython -m, hug -m). Specifying modules at the command line just isn't a common practice (yet :P). This is why I think we should flip your 1 and 2 (check if it's a file path that exists before checking if it's a valid module specification).

To clarify my hunch some more: I think people will use this with things they work on themselves more than with builtins.

Testing
One of the beautiful things about making CLIs w/ fire (imho) is that you aren't left with a hard-to-test main method. Naturally I think we should make this CLI w/ fire. We need two things to make this possible: 1. We need to finish issue #53. 2. We need to be able to move "import *" into a function call, and I think jtratner's comment will let us do this. Once we do this, we can write normal unit tests for this CLI.

Related to testing: internally at Google we have additional tests that do test Fire CLIs by running them as separate processes the way you'd normally run a CLI (that is, the tests use subprocess.Popen). I didn't open source those particular tests because the way the CLIs are built/referenced is google-specific.

@jtratner
Copy link
Contributor Author

jtratner commented Apr 8, 2017

@dbieber - I'm personally most interested in using this to work dynamically with libraries I've pip installed. Currently we do what fire does via some decorators with types similar to how click works, but I'd love to make building a CLI for a library to be equivalent to exposing the method at the top-level of the library API.

E.g.:

pip install counsyl-sequencer

fire sequencer -- --help

Or

fire sequencer.cli -- --help

Plus I could imagine some interesting use cases with pandas or with django hooking together an existing models file:

fire myproject.models MyModel objects get-by-id 21 - status

@dbieber
Copy link
Member

dbieber commented Apr 8, 2017

I'm personally most interested in using this to work dynamically with libraries I've pip installed.

Good to know. Maybe I'm wrong about how people will use this.

fire sequencer -- --help
fire sequencer.cli -- --help
fire myproject.models MyModel objects get-by-id 21 - status

I agree that all three of those commands should work exactly like that. The place where my proposal differs is that I think if there's a file in your current directory literally called 'sequencer', 'sequencer.cli', or 'myproject.models' then that file should get used instead of the module of the same name.


Aside: will that last example really work w/ django? That's really cool 👍.

@jtratner
Copy link
Contributor Author

jtratner commented Apr 8, 2017

I agree that all three of those commands should work exactly like that. The place where my proposal differs is that I think if there's a file in your current directory literally called 'sequencer', 'sequencer.cli', or 'myproject.models' then that file should get used instead of the module of the same name.

Oh totally true - agreed :)

@nealmcb
Copy link
Contributor

nealmcb commented Apr 8, 2017

When I quickly went thru system modules, I found fewer compelling use cases there than I had hoped. Using fire for testing should be popular, and -m is a familiar option. So those are good arguments for -m, and separating out file completion.

@dbieber
Copy link
Member

dbieber commented Apr 8, 2017

[Elaborating on how we could use Fire to write this.]

Ideally we could do something like this:

import fire

def launch_cli(file_or_module, *args):
  ...  # This is where we use imp to get the module.
  fire.Fire(module, args=args)

if __name__ == '__main__':
  fire.Fire(launch_cli)

This lets us avoid having to deal with sys.argv at all.

However there might be some issues w/ this approach:

  • which call to Fire gets the flags? (A: by default, the first call to Fire would get the flags, and only if there's additional flags and an additional '--' would the inner call to Fire get those flags. that's not ideal and we'd have to find a way around this).
  • we may need a workaround to prevent the arguments from being parsed prematurely. (decorators.py has such a workaround)

@dbieber
Copy link
Member

dbieber commented Mar 6, 2020

We're introducing the simplest possible version of this in Fire 0.3.0. This uses @jtratner's code from #110. Starting in 0.3.0, you'll be able to run python -m fire <module> <args> in order to use Fire with any module without modifying the code.

This includes builtin modules like calendar, installed modules (like fire itself!), and modules referenced relative to your current working directory.

So if you're in a directory with example.py, you can use fire on it by running python -m example <your-args-here>.

@dbieber dbieber closed this as completed Mar 6, 2020
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 a pull request may close this issue.

3 participants