tableprint icon indicating copy to clipboard operation
tableprint copied to clipboard

Custom print command

Open sumanthratna opened this issue 5 years ago • 4 comments

It'd be useful if tableprint could accept a custom print command. In my use case, I want to progressively (print-as-I-go) print the values of TensorFlow 2 tensors in a table.

In autograph, the only way to print TensorFlow tensors is with tf.print—using print(my_tensor) doesn't print the value.

I can think of two ways to approach this:

  • accept a callable that defaults to Python's print.
    • The only problem is that there are certain options that tableprint may need to pass to tf.print; for example, output_stream. Since these arguments differ from those of Python's print, tableprint's implementation will need to use if statements to see if the callable is tf.print. This brings me to the second option:
  • accept a boolean called print_tensorflow that defaults to False. If True, print using tf.print.
    • I like this idea better since the first option implies that any callable can be used, such as click.echo, even if tableprint doesn't have click.echo's options implemented (of course, a NotImplementedError could be raised).
    • However, this would limit tableprint to only print and tf.print. If you want to add more callables in the future, you could always change this argument to a string, and use an if statement to see if the string is python, tensorflow, click, etc.

sumanthratna avatar Apr 30 '20 21:04 sumanthratna

You can already pass a custom object for IO using the out keyword argument to table and TableContext. By default, this is sys.stdout, but it can be a file handle or any other object that has a write(str) method and a flush method. (This is exactly like the output_stream argument in tf.print).

I don't really want to add in custom logic for TensorFlow, but if you want the output to go through a tf.print call you can use a shim class like follows:

class TFWriter:
  def write(self, s):
    tf.print(s)
  def flush(self):
    pass

tp.table(data, headers, out=TFWriter())

nirum avatar May 16 '20 01:05 nirum

Thanks—the shim you wrote is pretty clever; I didn't think of that!

To be honest, I don't remember if I knew about the out argument—regardless, I should've mentioned that since I want to progressively print, I can't use out, as I need to use tp.header and tp.row, neither of which accept an out argument.

If you're okay with header and row accepting the out argument, I may be able to write a PR for this within the next two weeks.

sumanthratna avatar May 16 '20 01:05 sumanthratna

Happy to accept a PR for this!

nirum avatar May 16 '20 18:05 nirum

Currently, header and out both return strings, which need to be printed by the user. If we want to add the out argument, it makes sense to return None, like the other methods, and print using out. However, I'd consider this a breaking change.

We can:

  • make out default to None, and if out is not None, then use out to print; else, don't print anything and return a string. This avoids a breaking change, as users who currently don't use the out argument won't experience any problems, and I'll be able to use the shim you wrote. However, I'm not a fan of this—IMO, these functions should either print or return a string in all cases, to avoid user confusion.
  • make out default to sys.stdout, like the other functions. This means we should not return anything, and print using out. I like this behavior, as IMO it's more intuitive than the first option, but causes a breaking change, since users who don't currently use the out argument will be printing None along with tableprint's output.

What do you think? Honestly, I think tableprint is ready for a 1.x release, so I don't have a problem with causing a breaking change and bumping the major version.

sumanthratna avatar May 20 '20 20:05 sumanthratna