Display in Juno #902
Display in Juno #902
Conversation
|
I'm all for making Gadfly work nice with Juno, but Gadfly already installs a ridiculous amount of dependencies and I was hoping to reduce the number. Is there any way that we can make this work without enlarging REQUIRE? Maybe @tkelman knows? |
|
Yes. We need support for conditional imports in Base. (this was for you @ChrisRackauckas!) |
|
Three things here. 1, I think that "number of dependencies" shouldn't be the deciding factor. It should be whether having the dependency could cause breakage (i.e. the package has precompilation errors and is unstable) or namespace clashes, and additionally if the dependency is a huge install. None of those apply to Juno.jl. The only "problem" is it adds a number to the dependency count (and what, a few KB? May a MB?). I think the culture for evaluating dependencies would be much improved if this was the way it was discussed. Secondly, Gadfly is another case where conditional dependencies are starting to become a larger issue as the package grows. It seems anything that wants to extend itself with Juno support, support for units, extra plotting support, choices to use methods from different packages as "drop-ins" for native methods, etc. each require more and more dependencies. Gadfly has a lot of dependencies where are only used in a few cases like Loess which should really be conditional dependencies. I think this is a sign that the Julia ecosystem has reached a point where proper use of conditional dependencies is a big issue, and it needs to be addressed in Base. If you want to start pushing the discussion in Base, here are some places to do so: JuliaLang/julia#6195 I think the issue is that no one really knows how to deal with it, so any progress towards a PR would be huge plus. Lastly, note that Juno doesn't actually explicitly need to be a dependency, since Juno will always import Juno.jl on startup. So if you had |
| @@ -1105,6 +1105,22 @@ function display(d::REPLDisplay, ::MIME"application/pdf", p::Plot) | |||
| open_file(filename) | |||
| end | |||
|
|
|||
| # Display in Juno | |||
|
|
|||
| import Juno: Juno, @render, media, Media, Hiccup | |||
There was a problem hiding this comment.
are Media and Hiccup going to be dependencies of every version of Juno that will ever be released? if not, better to depend on then explicitly instead of importing them indirectly like this
There was a problem hiding this comment.
The indirect dependency on Media adds a dependency on Requires which is one of the known-to-be-unreliable conditional dependency workarounds.
There was a problem hiding this comment.
Yes, the idea is that the Media and Hiccup submodules form part of Juno.jl's public API. If that changes it can be handled in the standard way.
The whole point of this PR is to enable Requires.jl to be removed from Media.jl, and from the rest of Atom.jl's dependency tree.
There was a problem hiding this comment.
FYI @pfitzseb, something like in the Juno stack may have been the cause of the v0.5 upgrade precompilation error seen last night. Not this specifically, but if you used the same conditional dependency workaround somewhere else, it would lead to t and be fixed by the fix in the fix found in the issue:
|
@MikeInnes It looks like this PR is leading to precompilation errors on 0.4. Any thoughts on why that might be happening? I think working with Juno is important enough that I'm going to merge this once we figure out what's causing the precompilation problem.
@ChrisRackauckas I agree, but sheer number also matters because the fact of the matter is that upstream Julia packages break regularly*. The more potential sources of breakage the more likely Gadfly is to be broken due to a dependency that is only necessary for 1% of the function of the package. * and this is unlikely to change till we hit 1.0 |
|
The Juno 0.1.0 tag that claimed to support Julia 0.4 actually didn't. |
|
I tagged a new Juno.jl (JuliaLang/METADATA.jl#6474) which supports 0.4. After that's through the tests should pass again. |
Not according to the require file. Please pay closer attention to dependency requirements and testing on your packages. |
|
Tests are passing, and I've just removed Requires from Media.jl master – so that's one less dependency. |
|
Awesome! Thanks. Lets make Gadfly play nice with Juno. |
| media(Plot, Media.Plot) | ||
|
|
||
| @render Juno.PlotPane p::Plot begin | ||
| x, y = Juno.plotsize() |
There was a problem hiding this comment.
looks like much of the rest of gadfly uses 4 space indent
This adds a small, pure-Julia dependency on Juno.jl which enables Gadfly to be display appropriately in the Juno plot pane.