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

fix tests on julia v0.6-dev #158

Merged
merged 1 commit into from
Feb 20, 2017
Merged

fix tests on julia v0.6-dev #158

merged 1 commit into from
Feb 20, 2017

Conversation

rdeits
Copy link
Contributor

@rdeits rdeits commented Feb 17, 2017

This brings in enough updates to ensure that the unit tests pass on Julia v0.6-dev. I'm still having issues with actually using Interact in v0.6, but I think this is at least a step in the right direction.

With this update, I can execute an @manipulate cell in a notebook without errors, but the output of the cell does not update as I move the slider. I don't understand enough about the way IJulia actually works to be able to fix that yet.

One other note: this converts the nested functions in IJulia/setup.jl to anonymous closures to avoid world-age problems. That will hurt performance on Julia v0.4, so if you do go with this method, it might be worth bumping the julia requirement to v0.5 for these changes.

@JobJob
Copy link
Member

JobJob commented Feb 18, 2017

Thanks for this. I haven't tried it out but it looks good, nothing controversial afaict.

For mine I don't think performance issues on 0.4 are a major issue, but thanks for pointing it out. Pardon my ignorance, but what's the difference with the anonymous closures vs the previous?

Fyi I threw together some very rough notes a couple of months ago about the IJulia and Interact display code, hopefully IJulia hasn't changed too much since then: https://github.com/JuliaGizmos/Interact.jl/blob/master/doc/dev%20guide.md

@rdeits
Copy link
Contributor Author

rdeits commented Feb 18, 2017

re: anonymous functions, I don't really grok the new world-age system yet, but the current implementation resulted in errors caused by the fact that the handle_subscriptions function was too new to be called in the current context. I'm still trying to understand exactly what that means, but the anonymous functions trick is an easy way to avoid the issue here.

Thanks for the dev guide; it's exactly what I've been looking for!

@shashi
Copy link
Member

shashi commented Feb 20, 2017

I restarted the mac builds a few times, 0.4 went through 0.5 seems to hang. I'm merging this anyway.

@shashi shashi merged commit d3a2ba7 into JuliaGizmos:master Feb 20, 2017
@JobJob
Copy link
Member

JobJob commented Feb 21, 2017

I just tried this out in IJulia on yesterday's 0.6 with cells:

using Interact

Output: some deprecation warnings

s = slider(1:10)

No errors

signal(s)

world age related errors

I get the same errors regarding world age with or without this change - just the names in the error messages are clearer with the named functions. I'm curious, what situation(s) did the anonymous functions fix? I didn't get the impression from the docs that they should behave any differently to named functions/methods.

I had a look into it and it seems that we run into this situation on IJulia. Changing this line in IJulia to eval(Expr(:call, handlers[msg.header["msg_type"]], socket, msg)), and this line in Reactive to isrequired(a) && eval(Expr(:call, a.f, a.recipient.value, timestep))) and I can get signals to update in IJulia (just tested with the above). Oh I also had to add import Interact: Interact and change the displayable to Base.displayable in Interact/src/IJulia/setup.jl.

Apparently this is going to be merged which (I think) will mean we'll use invokelatest( instead of eval(Expr(:call, at which point we can make the necessary changes.

@JobJob
Copy link
Member

JobJob commented Feb 21, 2017

Also, it's far from impossible that I'm completely wrong about this world age stuff, I don't fully grok it yet either. @stevengj is it true that this line in IJulia will need to change to:
eval(Expr(:call, handlers[msg.header["msg_type"]], socket, msg)) (or
invokelatest(handlers[msg.header["msg_type"]], socket, msg) after your 19874 PR) since those handler callbacks will tend to be defined in code that gets loaded after that IJulia eventloop code?

@rdeits
Copy link
Contributor Author

rdeits commented Feb 21, 2017

@JobJob that's extremely weird. I'm not sure why it resolved the world age errors in my case, but I agree that your solution (using eval() and then eventually invokelatest()) makes more sense. Sorry for the incorrect patch.

@JobJob
Copy link
Member

JobJob commented Feb 22, 2017

Absolutely no problem at all, the contribution is appreciated.

@JobJob JobJob mentioned this pull request Mar 24, 2017
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.

3 participants