python-fire icon indicating copy to clipboard operation
python-fire copied to clipboard

Output of the example for grouping in docs is wrong

Open wchliao opened this issue 4 years ago • 3 comments

Hello.

In the Group Commands section (https://github.com/google/python-fire/blob/master/docs/guide.md#grouping-commands), the example code is

class IngestionStage(object):

  def run(self):
    return 'Ingesting! Nom nom nom...'

class DigestionStage(object):

  def run(self, volume=1):
    return ' '.join(['Burp!'] * volume)

  def status(self):
    return 'Satiated.'

class Pipeline(object):

  def __init__(self):
    self.ingestion = IngestionStage()
    self.digestion = DigestionStage()

  def run(self):
    self.ingestion.run()
    self.digestion.run()
    return 'Pipeline complete'

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

and according to the guide

$ python example.py run
Ingesting! Nom nom nom...
Burp!

However, the output is wrong. It should be

$ python example.py run
Pipeline complete

This can be easily fixed. However, in order to demonstrate the pipeline functionality, I would suggest to change the example to

class IngestionStage(object):

  def run(self):
    return 'Ingesting! Nom nom nom...'

class DigestionStage(object):

  def run(self, volume=1):
    return ' '.join(['Burp!'] * volume)

  def status(self):
    return 'Satiated.'

class Pipeline(object):

  def __init__(self):
    self.ingestion = IngestionStage()
    self.digestion = DigestionStage()

  def run(self):
    ingestion_output = self.ingestion.run()
    digestion_output = self.digestion.run()
    return ingestion_output + digestion_output

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

and accordingly the output should be

$ python example.py run
Ingesting! Nom nom nom...Burp!

I would like to know which change @dbieber would like. Thank you.

wchliao avatar Apr 23 '21 05:04 wchliao

Thanks for noticing the discrepancy. How about if we make Pipeline.run return a list [ingestion_output, digestion_output]. Then the stage outputs will be displayed on consecutive lines as in the guide's text.

dbieber avatar Apr 23 '21 12:04 dbieber

Sounds great! I'll make a change according to your suggestion.

wchliao avatar Apr 23 '21 19:04 wchliao

 ,

Sent from Yahoo Mail for iPhone

On Friday, April 23, 2021, 3:07 PM, Wei-Chung Liao @.***> wrote:

Sounds great! I'll make a change according to your suggestion.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Janeenm avatar Apr 23 '21 19:04 Janeenm