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

Push state in comm open msg #84

Merged
merged 4 commits into from
Jul 12, 2015
Merged

Conversation

jdfreder
Copy link
Contributor

No description provided.

@jdfreder jdfreder added this to the 4.0 milestone Jul 11, 2015
@minrk
Copy link
Contributor

minrk commented Jul 11, 2015

js tests appear to be failing.

@jdfreder
Copy link
Contributor Author

Hmmm strange, I'm really not sure why this is failing. I'll debug locally.

@jdfreder
Copy link
Contributor Author

Ahhh forgot sync=True on the model information traitlets! Should be good now!

minrk added a commit that referenced this pull request Jul 12, 2015
Push state in comm open msg
@minrk minrk merged commit d866fa7 into jupyter-widgets:master Jul 12, 2015
}).catch(utils.reject("Couldn't create a model.", true));
// For < 4.x
var model;
if (msg.content.data.model_name) {
Copy link
Member

Choose a reason for hiding this comment

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

In which case could this happen? Is this to handle an older version of the Python side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

How do you see this occurring ?

Copy link

Choose a reason for hiding this comment

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

the removal of this did break Interact.jl fwiw. probably good to have a deprecation for such things before breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we broke it in a later commit, sorry @shashi ! @SylvainCorlay and I will try to be better about things like this. It was removed in 474f08c

Copy link

Choose a reason for hiding this comment

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

I don't quite understand the whole process but that sounds like an awful lot of things to support. But yeah, if you want to do it, I won't say no :) I'd be happy if I had the good stack traces back, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that for each registered kernel, we should have the Javascript in a different directory, (one per installed kernelspec). The requirejs path would be automatically prefixed with a string corresponding to the current kernel.

I think this is a good idea and I'm +1 for it - it should be addressed with the other packaging problems https://jupyter.hackpad.com/Packaging-crate-PbIgxnC71or . @shashi with the feature @SylvainCorlay is describing, packaging would be more like npm. You, in the Julia widgets, would depend on a specific widget JS version X. That would allow us to continue chugging along without affecting you, and when you were ready to upgrade, you could.

@SylvainCorlay despite that, I still think for things like this, which are "widget msg spec" changing, we need deprecation warnings. Julia (@shashi) and Haskell (@sumitsahrawat) implementations both exist now, and we should do our best to help them transition to our latest Javascript.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know deprecation warnings can be a pain to keep track of, but I think they are much less of a pain for us than it is for our friends to migrate without them.

Copy link

Choose a reason for hiding this comment

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

Nice! Great to hear!

Copy link
Member

Choose a reason for hiding this comment

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

@jdfreder thanks, I will look at the hackpad.

@SylvainCorlay
Copy link
Member

@jdfreder this seems to be breaking a simple example such as

from ipywidgets import *
HBox([Button()])

@SylvainCorlay
Copy link
Member

Fixed in #86.

@jdfreder jdfreder mentioned this pull request Nov 6, 2015
@github-actions github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants