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

Backport #1958 to 4.x: Add an output callback override stack #2022

Merged
merged 1 commit into from
Feb 8, 2017

Conversation

jasongrout
Copy link
Member

@jasongrout jasongrout commented Jan 5, 2017

@minrk - what do you think about merging this in 4.x so that ipywidgets 6.x will work with notebook 4.x?

backport of #1958

Fix a subtle async bug in processing comm messages.

Basically, message processing did not wait for any promises a comm message handler may return to resolve, because comms were not set up to resolve return values of message handlers.

This allows us to override output callbacks to redirect output messages, and is used to implement the Output widget, for example. It does not redirect status messages or messages on other channels.

Write the get_output_callback_id function that we use.

Return after error condition.

Some tests for output callback overrides.
@jasongrout jasongrout changed the title Backport #1958 to 4.x: Add an output callback override stack WIP Backport #1958 to 4.x: Add an output callback override stack Jan 5, 2017
@minrk
Copy link
Member

minrk commented Jan 5, 2017

My hope was to not have any more new feature releases on 4.x, so we can push toward getting 5.0 out. But if there's a strong reason that this needs to be in 4.x rather than 5.0, that's okay, too.

jasongrout added a commit to jasongrout/ipywidgets that referenced this pull request Jan 6, 2017
@jasongrout
Copy link
Member Author

But if there's a strong reason that this needs to be in 4.x rather than 5.0, that's okay, too.

I think it depends on the timing of ipywidgets 6 vs notebook 5 - if notebook 5 is delayed for a long time after ipywidgets 6, I'd like to go ahead and get this in notebook 4.x. For now, we can leave the PR open for anyone that wants to run the ipywidgets beta on notebook 4.x.

jasongrout added a commit to jupyter-widgets/ipywidgets that referenced this pull request Jan 9, 2017
Make the output widget work in notebook 4.3.1 + jupyter/notebook#2022
@jasongrout jasongrout added this to the 4.4 milestone Feb 1, 2017
@takluyver takluyver modified the milestones: 4.4, 5.0 Feb 2, 2017
@jasongrout
Copy link
Member Author

@takluyver - can we still have a 4.4 milestone? Even if we release 5.0, a lot of people won't be able to update to it soon, and this makes it possible for ipywidgets 6.x to work in 4.x.

@takluyver
Copy link
Member

I'm hoping that the 5.0 release won't particularly be a more problematic upgrade than a new 4.x release. We're short enough on developer time here that I don't feel like we've got effort to spare for more 4.x releases.

@jasongrout
Copy link
Member Author

I'm hoping that the 5.0 release won't particularly be a more problematic upgrade than a new 4.x release. We're short enough on developer time here that I don't feel like we've got effort to spare for more 4.x releases.

I'm happy to take a crack at doing a very simple 4.4 that just has this backport in it.

@jasongrout jasongrout changed the title WIP Backport #1958 to 4.x: Add an output callback override stack Backport #1958 to 4.x: Add an output callback override stack Feb 3, 2017
@jasongrout
Copy link
Member Author

I'm happy to take a crack at doing a very simple 4.4 that just has this backport in it.

But I'll wait until we are closer to a widgets 6.x release - if it ends up taking a lot longer than 5.0, then it's probably a moot point.

For now, I'll make a 4.4 milestone again and put this PR in it just to keep track of it. We may decide to forgo the 4.4 after releasing 5.0.

@jasongrout jasongrout added this to the 4.4 milestone Feb 3, 2017
@SylvainCorlay
Copy link
Member

@gnestor I have tested this locally and it works well.

We are nearing the ipywidgets 6.0 release and this is a real blocker for us. Would you guys be OK with pushing a notebook 4.4 out so that we can iterate on widgets?

@SylvainCorlay SylvainCorlay merged commit 82b59d6 into jupyter:4.x Feb 8, 2017
@gnestor
Copy link
Contributor

gnestor commented Feb 8, 2017

@SylvainCorlay That's ok by me. @takluyver What say you? It seems like we are still at least a week out from a 5.0 release...

@takluyver
Copy link
Member

OK, if we must, but let's try not to get drawn into any more releases - it pulls away developer time, and we don't have very much of that for notebook now.

@ellisonbg ellisonbg mentioned this pull request Feb 8, 2017
6 tasks
@ellisonbg
Copy link
Contributor

@jasongrout @SylvainCorlay @gnestor have all been working really hard getting ready for for the 6.0 release of ipywidgets. Some aspects of this release as it relates to notebook releases:

  • Regardless of when notebook 5.0 comes out, we will need to release a 4.4 with the ipywidget fixes. Too many folks are still running notebook 4.x to not have latest ipywidgets work on it.
  • I think we are still a few weeks away from a 5.0 notebook release (see 5.0 Release Schedule #2150).
  • Releasing 4.4 should be much work as there hasn't been much work.

@gnestor can you take the lead on a 4.4 release? I opened an issue to discuss the 4.4 release #2161

@gnestor
Copy link
Contributor

gnestor commented Feb 8, 2017

@ellisonbg Yes, cutting a new release shouldn't take me more than an hour, so not a big deal.

@ellisonbg
Copy link
Contributor

ellisonbg commented Feb 8, 2017 via email

@jasongrout
Copy link
Member Author

Let's ping the mailing list on that too. I don't know of anything else.

@jasongrout
Copy link
Member Author

I'll write up a short paragraph for release notes for this pr

@gnestor
Copy link
Contributor

gnestor commented Feb 8, 2017

@jasongrout If you put the release notes here, I will add them to the changelog for you 👌

@SylvainCorlay
Copy link
Member

@ellisonbg @gnestor thanks a lot for this.

@gnestor
Copy link
Contributor

gnestor commented Feb 8, 2017

@jasongrout Better yet, comment on this PR regarding releast notes: #2163

gnestor pushed a commit that referenced this pull request Feb 9, 2017
Backport #1958 to 4.x: Add an output callback override stack
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants