Option to print help to std out

[cbrochtrup]

Thanks for creating Fire. It's a fantastic argument parsing tool.

A minor request: I prefer my help strings to be printed to stdout instead of a console like less. When the output is sent to a less-like program you can't reference the script argument information when trying to type your next command.

The naive way to allow this option is adding a switch so the user can choose to send information console package or to stdout (code below).

https://github.com/google/python-fire/blob/d77453938a4b7a5a2bb71d9cb40397ee8bbc2e0a/fire/core.py#L171 to something like

if print_std:
   print(text, file=out)
else:
   console_io.More(

I don't see an elegant way to set print_std in your source code since you are not accepting contributions to your console package and the Display function is not a method of the Fire class. I don't see a straightforward way to have the user set this display option. Obviously, you know the code better than me so you may know a good way to add this option.

I am using fire version 0.2.1 and have seen this behavior with Ubuntu 16.04 and 18.04.

Thank you for your time!

[dbieber]

Thanks for this feedback. Let me think on this some and get back to you.

[nikking]

One example to look at is git. The default for --help is stdout, but for commands that are expected to be longer than a page (e.g. git diff), the pager is used.

Git also allows you to adjust the behavior with options (--no-pager), an environment variable (GIT_PAGER), and configuration (core.pager).

Sources: https://www.git-scm.com/docs/git/1.7.4 and https://www.git-scm.com/docs/git-config

[jaredtrog]

Any progress on this enhancement? I'd be happy to do the work if @dbieber or other members have some guidance on how they'd like to see this feature implemented.

It looks like @cbrochtrup may not be too far off on the implementation since More() currently calls out.write if the session is not interactive. Perhaps a function in console_io called StdOut that more or less mirrors More? It could handle checking terminal capabilities to ensure ANSI escape sequences are supported or bail in a more graceful way.

At that point the environment functionality @nikking mentioned could be implemented in core.

[nlarusstone]

This is a feature I would really appreciate! I have been enjoying the simplicity of Fire so far, but when you have even a couple steps of nesting it quickly gets frustrating to keep bringing up a pager and having to remember all the arguments. I would love to have my history printed to std out so that I can refer back to it.

Let me know if I can help at all!

[dbieber]

Here's how I think about this. We want all of the following:

1. Option to disable output altogether (#231)
2. Option to print help to stdout instead of pager (#188)
3. Ability of user to provide custom output serialization (#74)
4. Sensible defaults (always!)
5. Clean API that will make sense even as we add more configurations (#98)

So, for example, one API we could consider is: fire.Fire(display='less') Where valid options for display are ['print', 'less', 'none'] This gives 1., 2., ~3., hopefully 4., but doesn't do a great job of 5. It should be clear how it does 1 + 2. It does ~3 a little because the user can pass 'none' and then get the value returned by fire.Fire, and use it to print whatever they want. However, the user shouldn't have to forgo Fire's error handling to get custom serialization. And it doesn't do a great job of 5 because a) the string 'none' is confusing, b) the string values are all different types of things -- 'print' is a python builtin fn, 'less' refers to a process, and 'none' is a categorical choice.

Let me know if I can help at all!

Yes! Can you suggest an API that meets these 5 criteria?

[cbrochtrup]

Just spitballin' here (this may be dumb) but could Display be a method of the Fire class that can be overridden by the user? The API would be fire.Fire(display_func=custom_function) and this would also require moving _DisplayError and _PrintError into the Fire class or giving them an extra display_fucn argument. This would give the user a lot of flexibility with the output but probably cause an influx of issues where people couldn't get their custom print functions to work :sweat_smile: It may be too much flexibility?

I'm willing to try this if you approve.

[dbieber]

Definitely not dumb :)

What would be the signature of display_func? Would it accept an arbitrary object that it is responsible for both serializing and displaying? Or would it just accept a string and it is only responsible for displaying it?

[cbrochtrup]

I was thinking it'd be identical to Display in that it must accept a string and a stream, display_func(string, out). The custom function would be responsible for serializing and displaying (as long as serializing means formatting the string, I may not know everything serializing entails). I think an example in README.md will give most everyone what they want and the interested developer can still fiddle the output to their hearts content.

Now users could do

def simple_display(lines, out):
  text = '\n'.join(lines) + '\n'
  print(text, out=out)

fire.Fire(display_func=simple_display)

Something like this would switch between the user and default

  self._Display = Display
else:
  # Code to make sure display_func is valid
  self._Display = display_func

and then call self._Display everywhere Display is currently called.

[dbieber]

Serializing refers to going from the final component to the output string representing it, as in _PrintResult.

I was thinking it'd be identical to Display in that it must accept a string and a stream

One thing to consider is that Display is only one of a few places that Fire displays output today. The others are the various calls to print( in core.py outside of Display, such as in _DisplayError and _PrintResult. At the moment I count 9 calls to print, 3 to stderr and 6 to stdout.

[nlarusstone]

What if instead of passing in a function like display_func, we passed in a class instead. We could have a default display class that looks something like:

class Display:
   def serialize():
      ....
   def print():
     ....
   def error():
    ....

We would provide a couple of classes that users can import as follows:

from fire.display import Display, Less, Stdout

and then use as follows:

fire.Fire(display=Stdout)

This has the advantage of centralizing all of the display code, as well as making it clear what interfaces someone needs to implement if they want to build a custom output tool.

Option to disable output altogether -- we let users pass in None Option to print help to stdout instead of pager -- we provide an implementation of a class that does this Ability of user to provide custom output serialization -- user can pass in their own class as long as it conforms to the Display interface Sensible defaults -- default would be same as it is currently (i.e. Less class) Clean API that will make sense even as we add more configurations -- As long as we agree on a sensible API for the Display class, it should be extensible.

Thoughts?

[jaredtrog]

A class like Display as outlined above could implement a serializer that handles all sorts of different values but then a new StdOut class would either need to do an import from Display's serialize method or copy/paste or write new code to serialize. That's not great if the only goal is to change the output destination.

There appear to be at least four steps occurring to get something to print out; generation, serialization, formatting, and output.

I think one of the main challenges is how tightly coupled the four steps are.

The function _PrintResult is a good example. In some cases it does it's own serialization (prints the result as a string) and in other cases it calls out to a function that serializes, like _DictAsString, then outputs the result. In a third case it it sends text out to a separate function that in turn provides output (the call to Display on L269). So the function name might indicate it's just providing output but really it's doing a lot of work. _DisplayError is also a good one since it directly calls out to formatting, prints output, and calls other output generators in the same function.

I'm sure everyone else on this thread is way ahead of me but some of the questions that crossed my mind are: If you wanted to insert code to globally disable formatting, where would it go? Or, as is under discussion, if you wanted to change output destination, where would it go? As @dbieber pointed out, there isn't a central place.

Separating out those three (since formatting is already separate) functional systems into discreet components seems like a good direction but also a reasonably challenging one.

Some of the challenges I think would need to be solved to move this direction.

1. Refactor all print statements into calls to an "output" function or methods on an output class. It may not make sense to reuse Display since it's hardcoded to More right now and probably not _PrintResult since it's doing a lot of mixed work. The "output" function could call Display though.
2. Refactor all output redirection into the "output" function or class. Nothing outside the "output" function should refer to stdout, stderr, etc.
3. Refactor all statements that serialize data into calls to a "serialize" function or methods on a serialize class. There is a comment in core.py on L243 that seems to indicate this was the planned direction.

As more defined paths emerge in generation, serialization, formatting, and output then I think the spec for a scalable, generally applicable API will emerge.

As the code evolves the main entrypoint might look something like this:

# core.py
from output import More
from serialize import FireToStr
# Maybe make formatting a bool?
...
def Fire(component=None, command=None, name=None, serializer=FireToStr, formatting=True, output=More):
...

I may be over thinking the situation - Does this seem too deep of an approach for an otherwise simple problem?

In the meantime, optionally shunting everything that would have got to the console_io.More function to stdout may be useful though I understand avoiding temporary solutions that are almost guaranteed to break in a future release.

[nlarusstone]

@jaredtrog I think you make some great points, but let me put my OOP hat on for a second to see if we can meet in the middle.

A class like Display as outlined above could implement a serializer that handles all sorts of different values but then a new StdOut class would either need to do an import from Display's serialize method or copy/paste or write new code to serialize. That's not great if the only goal is to change the output destination.

I was implying that StdOut could inherit from Display, so it wouldn't need to implement the serializer -- just the output. Something like:

class StdOut(Display):
  # Other methods are implemented by DisplaY
  def output():
    ....

There appear to be at least four steps occurring to get something to print out; generation, serialization, formatting, and output.

I like this separation and think that whatever the solution it is, it should conform to this API.

def Fire(component=None, command=None, name=None, serializer=FireToStr, formatting=True, output=More):

One of the reasons that I'm suggesting a class instead of passing arguments like this is that it better supports customization. With an API like you've suggested above, what happens when I want to customize the entire display system? Do I need to implement a class for each step? What about customizing just formatting and output?

Logically, all 4 of these steps -- generation, serialization, formatting, and output -- are conceptually very related, so I think it would be nice if we could handle them together.

[jaredtrog]

I think the middle is where the best solutions are found 👍.

Let me restate what I think you're saying to make sure I'm responding to the ideas as stated.

1. Develop a class called Display that has methods to perform serialization, formatting, and output.
2. Pass the class into the main entry point Fire function with the keyword argument display.
3. Rework the existing fire code base to replace print statements with something like display.print, display.Error, etc.
4. Rework the existing fire code base to replace serialization calls with something like display.DictAsString, display.OneLineResult, etc.
5. If a user wants different results, they would subclass Display, overwrite whichever method needs to perform differently and pass it to their invocation of Fire with something like display=StdOut.

Is that accurate?

[dbieber]

I like the idea of separating out these different components.

Strawman idea

Here's the idea I'm toying with at the moment, but it still has some (big) drawbacks.

Signature: def Fire(component=None, command=None, name=None, serialize=DEFAULT, print=DEFAULT, display=DEFAULT):

Here's how you would do each of 1-4:

1. (output altogether) Fire(print=None, display=None)
2. (stdout instead of pager) Fire(display=print)
3. (custom output serialization) Fire(serialize=custom_serialization_fn)
4. (sensible defaults!) Fire() (uses today's behavior)

The signatures for both print and display would be fn(str, stream) (same as Python's builtin print today).

A few (maybe major) drawbacks:

1. print is a keyword, so using it as an arg name is a bit unorthodox, and not even valid Python 2 syntax.
2. The distinction between print and display is clear(ish) when using the defaults (print is for printing to stdout, display is for using a pager like less), but doesn't make as much sense when thinking about them just as two ways of showing output. E.g. it won't be obvious to a user what they need to do if they just want to change how errors get printed. Or just want to change how objects are printed. Hopefully it will be clear what to do if you just want help to be printed instead of shown in less though.

Also, I think we would omit serialize in the first pass; we already offer one way of doing custom serialization (#74). But it's good to know we would have a consistent way of introducing it to the API at a later point. ("Consistent" because it fits the same naming scheme as display and print (imperative verbs) and because like display and print, it also accepts a function with a simple(ish) signature.)

---

Thoughts on configuring with a class

One concern I have about using a class for configuration is that the amount of boilerplate is high (at least by Fire's standards). A user would have to import a base class, create a new class, create a new function, and then include their (likely one-liner) function definition. Another concern is that it makes the API less visible. So far the API is visible by looking at fire.Fire's function signature. If one of the args accepts a function, that already makes the API slightly opaque (because the user needs to read the docstring or documentation to determine the expected function signature to pass in), but if an arg accepts a class then that makes the API even more opaque (because now the user doesn't just need to look up a function's signature, which they might have been able to guess if it's simple enough; now they need to look up the specs required for a class they're passing in.)

(Tying this back to the 5 criteria above, I think using a Display class for configuration risks harming goal 5 of having a clean api.)

---

Signature of config functions

Here's another thing I'm thinking about:

Let's say we accept a function display(text, stream). Can we also accept display(text)? I think we can, just by using inspection to see if it accepts one or two arguments before calling it. This allows people to write cleaner code if they don't care about the stream (e.g. because they always want to write to stdout). It makes the documentation a little messier, because we have to describe that we accept either kind of function, but I think that would be worthwhile.

[nlarusstone]

@jaredtrog That's exactly what I was suggesting, thanks for putting it so succinctly.

@dbieber I understand your concerns, but I think that any solution we decide upon is going to increase the complexity of the API, since we're trying to handle a few new usecases as well as allow users to implement arbitrary functionality. That being said, I think one of the best parts of Fire is its minimalist design/API, so we should definitely keep our solution as sparse as possible.

One concern I have with just placing functions into the fire.Fire API is that it is not clear how they interact or what constraints they have. If I implement a custom serialization function, does that work with the default print/display functions (and vice versa)?

I'm also not sure I understand the distinction between display and print that you've laid out above. Why couldn't we do 1. (disable output altogether) Fire(display=None) ?

[dbieber]

Why couldn't we do 1. (disable output altogether) Fire(display=None)?

Agreed that's another drawback to the strawman proposal. It would definitely be good to have a solution that allows turning off all output with a single setting.

[dbieber]

That suggests another candidate: fire.Fire() -> defaults (print usually, but use pager for help) fire.Fire(display=fire.DefaultDisplay) -> defaults again fire.Fire(display=None) -> turns off output entirely fire.Fire(display=print) (Python3 only) -> print everything, never using less fire.Fire(display=fire.Print) (Python2/3 compatible) -> print everything, never using less

def display(text, file=None, kind=None):
  ...
fire.Fire(display=display)

where kind is one of ['output', 'error', 'usage', 'help']

It would be nice to trim this down to just def display(text, kind=None): because if the user's writing their own display function, they don't necessarily care which stream Fire would have written the output to. They can write their output anywhere they like. It would also be nice to support display=print in Python 3 though, which has a different signature (print(value, ..., sep=' ', end='\n', file=sys.stdout, flush=False)). Maybe we can support both via inspection: e.g. the rule could be "if display just accepts one argument, call display(text); if it accepts multiple arguments and there's a kwarg named 'file', we pass stdout/stderr as appropriate; and if there's an argument named kind, we pass it the output kind." It's a drawback to require the user to name their args file/kind though, but maybe not a huge deal.

[nlarusstone]

I like that proposal and think that if the user is writing a custom display function, it's ok for them to conform to our expected API (naming args file/kind).

Do we need to account for the serialization usecases as well though?

[dbieber]

Yeah, serialization would be a separate config under this proposal. fire.Fire() -> defaults fire.Fire(serialize=fire.DefaultSerialize) -> defaults again

def serialize(component):
  return repr(component)  # or whatever the user decides
fire.Fire(serialize=serialize)

[dbieber]

Maybe also we would use fire.Display and fire.Serialize as the defaults (instead of fire.DefaultDisplay and fire.DefaultSerialize) to keep things cleaner.

[An advantage of DefaultDisplay/DefaultSerialize though is that it allows Fire to offer built in alternatives if we later want to (like fire.SerializeCompact or something...) though idk if we'd want that. I guess fire.Serialize doesn't preclude that either...]

[nlarusstone]

Sorry, I think I'm getting a bit confused here. Are fire.Display and fire.Serialize classes or functions?

I think display and serialize are fine without the default prefix.

[dbieber]

Functions

[dbieber]

fire.Fire is a function too.

[dbieber]

The reason we use FunctionNames instead of function_names is that at the time this project was created, that was an acceptable standard by the internal google python style guide. We've continued doing it for consistency. It would be good to switch to the more accepted standard of snake_case function_names. We have to keep supporting fire.Fire of course though, since that's used in thousands of projects already...

[nlarusstone]

Ok, I'm still a little worried about the fact that we will need to add an argument per operation. What happens when we want to add a step between serialization and display (e.g. formatting)?

[dbieber]

Let's do our best to anticipate things we may want to add in the future. Can you give an example of what formatting might be used for? To me it sounds like serialization and display cover the full pipeline from output to eyeball so I don't think a separate formatting step would be needed.

[dbieber]

[Brainstorming: Another somewhat related place we may end up wanting an additional argument is exception handling. Fire raises FireExit exceptions (which extend SystemExit) in order to exit with a particular exit code. Users can catch these exceptions to prevent Fire from exiting the program, but it's possible we'd want to add a config around this too.]

[nlarusstone]

Maybe I misunderstand what serialization entails (I would have thought it involves taking the information to be output and packaging it into a single, easily transmitted bundle).

Formatting to me would then be the process of arranging that information on screen (and display would be where to show it). An example of formatting would be if you're displaying on a pager, you might want to format with extra line breaks between arguments to improve readability. Or that you might want to list the parameter shortcuts (-x) in a different place from the full parameter names (--xample)

[albertotb]

Coming from #231, I think the proposed fire.Fire(display=None) would be a great way to catch the return of the function. As I understand it, this file test.py

def fn_test(a, b):
   return a + b

if __name__ == "__main__":
    ret = Fire.Fire({'fn_test': fn_test}, display=None)
    print(ret)

when called like

python test.py fn_test 5 6

would print 11 only once and it would be exactly the same as if fn_test(5, 6) was called, right?

Also not related to #231, IMHO I'd rather pass functions to control output than classes, since they can be written on the spot, but I'm not familiar at all with the internals.

[fabioz]

As a note, until there's a proper fix, the following workaround does seem to work (on version 0.2.1):

if __name__ == "__main__":
    def Display(lines, out):
        text = "\n".join(lines) + "\n"
        out.write(text)

    from fire import core
    core.Display = Display
    fire.Fire(...)