Skip to content

Commit

Permalink
Don't strictly require IJulia
Browse files Browse the repository at this point in the history
  • Loading branch information
shashi committed Jan 26, 2016
1 parent 2b1c78e commit 3cdbb0f
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/GadflyDiff.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ function writemime(io::IO, m::MIME"text/html", ctx::Signal{Gadfly.Plot})
), c), ctx))
end

IJulia.metadata(::Reactive.Signal{Plot}) = Dict()
using Requires

This comment has been minimized.

Copy link
@tkelman

tkelman Jan 26, 2016

Would need to be listed in REQUIRE. This package is likely to break when jb/functions lands.

This comment has been minimized.

Copy link
@shashi

shashi Jan 26, 2016

Author Owner

ComposeDiff requires Requires, so I thought this is not required in REQUIRE. Is that kind of thinking bad?

This comment has been minimized.

Copy link
@shashi

shashi Jan 26, 2016

Author Owner

Why will it break with jb/functions? Will all packages requiring Requires break?

This comment has been minimized.

Copy link
@tkelman

tkelman Jan 26, 2016

Only if ComposeDiff will always be a dependency of this package and Requires will always be a dependency of ComposeDiff. Better to be explicit and list everything you use, as opposed to relying on transitive deps since dependencies change over time. Particularly the Requires package is not a great idea to rely on w.r.t. precompilation and generally hooking into things that shouldn't be messed with - isn't the point of splitting things up to do this properly?

This comment has been minimized.

Copy link
@shashi

shashi Jan 26, 2016

Author Owner

Okay will add Requires to REQUIRE.

Yeah, Requires and similar approaches do not play well with precompilation. It really bothers me that there is no real substitute to Requires now, you need to do a hand-stand like this and make it work. This kind of splitting up of things means N^2 more packages for connecting N packages up, so it's not ideal. This is frequently needed in UI packages.

What I really am going for is installing Escher and using Gadfly in it should not also require IJulia because ComposeDiff requires IJulia...

This comment has been minimized.

Copy link
@tkelman

tkelman Jan 27, 2016

Or you try to design a common API that covers various use cases - see for example MathProgBase, which is designed for (and does a very good job of) solving essentially the same N^2 interfaces problem but for a very different application domain.

This comment has been minimized.

Copy link
@shashi

shashi Jan 27, 2016

Author Owner

I think the extern keyword suggestion in JuliaLang/julia#6884 could help deal with this.

Not sure this one method would merit an IJuliaBase (or some such) package, there's not much other stuff that can be abstracted out.


@require IJulia begin
IJulia.metadata(::Reactive.Signal{Plot}) = Dict()
end

function __init__()
for f in Requires.__inits__
f()
end
end

end # module

0 comments on commit 3cdbb0f

Please sign in to comment.