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

Message hook in display publisher #115

Merged
merged 10 commits into from Mar 31, 2016
Merged

Message hook in display publisher #115

merged 10 commits into from Mar 31, 2016

Conversation

dwillmer
Copy link
Contributor

This is the initial work for #113.

  • unregister hook.
  • unit-tests for both return possibilities from hooks.
  • for the rare cases with multiple hooks, install new ones at the front, rather than the back?

@dwillmer
Copy link
Contributor Author

pinging @minrk @jasongrout

@@ -70,6 +78,15 @@ def _flush_streams(self):
sys.stdout.flush()
sys.stderr.flush()

@default('thread_local')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I think this is the first PR that uses the traitlets 4.1 API, you may want to bump the version requirement in setup.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Master is already at 4.2 -

https://github.com/ipython/ipywidgets/blob/bb5092be3c0e75a12293472584a218d9a217c935/setup.py#L108

so i'll just merge on that.

Thanks!

@minrk
Copy link
Member

minrk commented Mar 25, 2016

Nice start!

@dwillmer dwillmer changed the title [WIP] Message hook in display publisher Message hook in display publisher Mar 30, 2016
@dwillmer
Copy link
Contributor Author

@minrk @jasongrout now has comprehensive tests over the new hook functionality - your call on whether we merge and iterate or carry on.

-------
None

The DisplayHook objects must return a message from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these are DisplayHook objects since DisplayHook is called on objects, not messages. I think it's any callable with:

def hook(message) => message or None

@minrk
Copy link
Member

minrk commented Mar 30, 2016

Awesome, thanks @dwillmer! I made one comment in-line about DisplayHook, but I think we can merge and move on shortly.

@dwillmer
Copy link
Contributor Author

@minrk - Fixed, thanks!

@minrk minrk added this to the 4.4 milestone Mar 31, 2016
@minrk minrk merged commit 783a55d into ipython:master Mar 31, 2016
@minrk
Copy link
Member

minrk commented Mar 31, 2016

Thanks!

maartenbreddels added a commit to maartenbreddels/ipykernel that referenced this pull request Apr 14, 2023
Similar to ipython#115 but for stdout/stderr

This can be useful for a thread to redirect stdout/stderr to a
file, while other threads can still write to stdout/stderr.
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 this pull request may close these issues.

None yet

2 participants