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

Add @capture decorator to output widget #1934

Merged
merged 12 commits into from Mar 14, 2018

Conversation

Projects
None yet
3 participants
@pbugnion
Member

pbugnion commented Jan 26, 2018

This PR adds a @out.capture decorator that pushes all the output of a function to an output widget. It addresses issue #1846 .

For instance,

out = widgets.Output()

@out.capture
def f(arg):
    print(arg)

f('argument')

out.outputs
# => ({'name': 'stdout', 'output_type': 'stream', 'text': 'argument'},)

This is useful for capturing output from widget callbacks:

b = widgets.Button()
b.on_click(f)
# everytime someone clicks on `b`, `out.outputs` gets appended to.

To do:

- [x] Documentation on output widget (probably in a separate PR) (moved to issue #1935)

  • Use functools.wraps to avoid losing the function name and docstring (https://docs.python.org/3.6/library/functools.html#functools.wraps)
    - [ ] Is there a better name than out.capture? (I'm comfortable with out.capture).
    - [ ] Should current outputs be cleared on entry? (I think that would be more confusing -- we could add a recipe to the documentation for creating an output widget with a clear button).

@pbugnion pbugnion changed the title from Add @capture decorator to output widget to [WIP] Add @capture decorator to output widget Jan 26, 2018

@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Jan 26, 2018

I like it 👍. I think capture is a good name. An alternative would be redirect, coming from redirecting stdout/stderr, but I like it less than capture.

pbugnion added some commits Jan 26, 2018

Avoid changing wrapped descriptors
By using functools.wraps, we avoid overwriting the __repr__ and __doc__ of the
wrapped callback.
@pbugnion

This comment has been minimized.

Member

pbugnion commented Jan 26, 2018

Thanks!

I think capture is a good name

I tend to agree. Jason isn't so keen. An alternative could be slurp, but it's pretty ugly.

@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Jan 26, 2018

What about making it callable?

@out
def f(arg):
    print(arg)
@pbugnion

This comment has been minimized.

Member

pbugnion commented Jan 26, 2018

What about making it callable?

So replacing .capture with .__call__? I personally think that's a bit less explicit, but happy to implement it that way if we think that's the right way.

@pbugnion

This comment has been minimized.

Member

pbugnion commented Jan 26, 2018

Another possible question is -- we could clear the current outputs at each new entrance to func. This brings it a bit closer to how an output cell works: the user just sees the result of the last execution of func.

@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Jan 26, 2018

Indeed, I think the name capture is self documenting, maybe @out.redirect_output even more. It's longer, but we have a tab key for a reason :).
Now that I think about it, do we want to redirect stderr/stdout to seperate places maybe, how does the output widget do it now? From a user point of view this may seen nice:

  • @out.redirect_all
  • @out.redirect_stderr
  • @out.redirect_stdout
@pbugnion

This comment has been minimized.

Member

pbugnion commented Jan 26, 2018

how does the output widget do it now

The outputs trait is a tuple of each output type -- you get entries for display_data, error, output streams etc:

@out.capture
def f():
    """hello"""
    print('hi')
    display(HTML('<p>hello</p>'))
    raise Exception('bla')

f()

# =>

({'name': 'stdout', 'output_type': 'stream', 'text': 'hi\n'},
 {'data': {'text/html': '<p>hello</p>',
   'text/plain': '<IPython.core.display.HTML object>'},
  'metadata': {},
  'output_type': 'display_data'},
 {'ename': 'Exception',
  'evalue': 'bla',
  'output_type': 'error',
  'traceback': ['some long traceback']})

Given the number of output types, I'm not sure there'd be a strong use case for redirecting some outputs but not others. That behaviour would also be substantially different to the current context manager.

@pbugnion pbugnion changed the title from [WIP] Add @capture decorator to output widget to Add @capture decorator to output widget Jan 26, 2018

@pbugnion

This comment has been minimized.

Member

pbugnion commented Jan 27, 2018

I think this is ready for review and merging as is.

@jasongrout

This comment has been minimized.

Member

jasongrout commented Feb 13, 2018

What if we made it output.capture(clear=True) for the automatic clearing case? We could even put in the automatic matplotlib magic that @interact does.

We could support both @output.capture and @output.capture(clear=True) by checking the first argument. We could also change things to be @output.capture() for the base case. Or we could do something like what @jdemeyer did for interact, and do

@output.capture.options(clear=True)
def f():
    ...
@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Feb 13, 2018

I don't like the .option, but the optional keyword seems pretty clean!

@jasongrout

This comment has been minimized.

Member

jasongrout commented Feb 13, 2018

Once we move to python 3, it will be even cleaner since we can insist that clear=True is a keyword argument.

@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Feb 13, 2018

Is it maybe a plan to put in comments in the code, that we can grep to make that easy? # PY3: ... orso

@pbugnion

This comment has been minimized.

Member

pbugnion commented Feb 13, 2018

Thanks for the feedback -- I'll add a clear=True keyword to the decorator, with a PY3 style comment to remember to switch to enforcing a keyword argument when we move to python 3.

@pbugnion

This comment has been minimized.

Member

pbugnion commented Feb 25, 2018

The decorator now supports clear_output=True. Setting this to True clears the output with every new invocation. -- I went for clear_output as the argument name, instead of clear, to remain consistent with the clear_output method name. Any extra *args and **kwargs passed to the decorator are passed onto the clear_output call, so users can use wait=True.

By default, clear_output is False -- I think this is more consistent with the behaviour of the context manager, which doesn't clear the output every time it is entered.

capture-decorator-example

This is ready for another review.

pbugnion added some commits Feb 25, 2018

Default clear_output to False
This is more consistent with the behaviour of the context manager, which does
not clear the output every time it is entered.

@jasongrout jasongrout merged commit 3526180 into jupyter-widgets:master Mar 14, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@jasongrout

This comment has been minimized.

Member

jasongrout commented Mar 14, 2018

Thanks!

@pbugnion pbugnion deleted the pbugnion:output-widget-add-capture-decorator branch Mar 14, 2018

@pbugnion

This comment has been minimized.

Member

pbugnion commented Mar 14, 2018

Great, thank you!

@maartenbreddels

This comment has been minimized.

Member

maartenbreddels commented Mar 14, 2018

Yeah, great work!

@jasongrout jasongrout modified the milestones: Minor release, 7.2 Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment