search

JuliaDocs / DemoCards.jl

Let's focus on writing demos
MIT License
65 stars 14 forks source link

Make Pluto dependency optional? #130

Closed frankier closed 1 year ago

frankier commented 1 year ago

Would it be possible to make the Pluto dependency optional? Currently there is a problem if we want to have WGLMakie installed along with DemoCards.

WGLMakie depends upon JSServe depends upon WebSockets, depends upon HTTP < 1 But Pluto depends upon HTTP > 1

See https://github.com/SimonDanisch/JSServe.jl/issues/131

frankier commented 1 year ago

@Dsantra92

Dsantra92 commented 1 year ago

It can be done, but having HTTP < 1 looks a bit unreasonable to me. Can you confirm this the issue with Makie.jl with WGLMakie as backend?

Dsantra92 commented 1 year ago

Looks like JSSserve.jl supports HTTP 0.8 and 0.9 only. But according to https://github.com/SimonDanisch/JSServe.jl/issues/131 next version of WebSockets will use 1.0+. So let's just wait till then? I don't think there will be any major updates in julia and markdown formats for now. So you can pin DemoCards.jl version until the release of WebSockets.jl.

frankier commented 1 year ago

Okay. Seems reasonable. Thanks!

frankier commented 1 year ago

I still think this would be a nice idea. I guess most people will not use the Pluto support, but as it is now, these projects get all of Pluto pulled in as dependencies into their docs. Note that currently Literate.jl does not have very many dependencies, and notably doesn't pull in any Jupyter stuff, it just generates the format as JSON. Adding Pluto as a dependency for all uses of this package could significantly increase TTFD (time to first doc).

t-bltg commented 1 year ago

What's the status of this ? This is a problem too for https://github.com/JuliaPlots/PlotDocs.jl.

Optional dependencies should use @require from https://github.com/JuliaPackaging/Requires.jl.

Dsantra92 commented 1 year ago

Hi, what is the current problem with PlotDocs.jl? Time to first plot?

t-bltg commented 1 year ago

Compat, can't update to latest DemoCards.

EDIT: and also, dependency hell :) We have to be extremely cautious when introducing heavy dependencies.

Dsantra92 commented 1 year ago

Looks like more of a WebSockets.jl issue here. Track here https://github.com/JuliaWeb/WebSockets.jl/issues/181

Dsantra92 commented 1 year ago

While I do recognize the time to first plot a good viable reason, I think moving a library to optional because another library release is taking time to release the next version is a bit of a hassle. Let's wait for a week and see how then plan on how the next release goes!

frankier commented 1 year ago

This could continue to be a compatibility problem later on as different parts of the ecosystem continue to be developed at different paces. It is not obvious why DemoCards.jl needs to depend upon Pluto if you are not using this feature.

t-bltg commented 1 year ago

What is @johnnychen94's opinion on this matter ?

johnnychen94 commented 1 year ago

There are two issues to solve here:

I do see people having the need to remove Pluto dependency completely from the current Project.toml. One way is to:

How would you guys think?

t-bltg commented 1 year ago

It is unclear to me why https://github.com/JuliaDocs/DemoCards.jl/pull/121 needs to pull-in Pluto and PlutoStaticHTML.

Maybe @Dsantra92 could clarify.

Instead of adding using Pluto, one should use import Pluto: ... so that it is immediate to see which components are effectively used.

I mean by this that trivial helpers such as is_pluto_notebook could easily be duplicated here ...

To me using Requires would be acceptable (you could still manage the Pluto dependencies in Project.toml in the [compat] sections).

I may be easy to fix, otherwise I would agree to the 0.4 - 0.5 reversal and branching.

More evolved strategies would be introduced by https://github.com/JuliaLang/julia/pull/47040 or https://github.com/JuliaLang/julia/pull/47695, but that is long term.

hustf commented 1 year ago

WebSockets.jl now has a new version, requiring Julia 1.8.2+ and HTTP 1.1.0 to 1.5. Hopefully, one plug is unstuck.

Dsantra92 commented 1 year ago

I think it is better to make the dependency optional for the time being. Pluto and PlutoStaticHTML are not maintained in sync, resulting in bugs more often than anyone would like. Thanks, @t-bltg and @frankier for the suggestion.