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

Make Pluto dependency optional? #130

Closed
frankier opened this issue Nov 5, 2022 · 16 comments · Fixed by #138
Closed

Make Pluto dependency optional? #130

frankier opened this issue Nov 5, 2022 · 16 comments · Fixed by #138
Assignees

Comments

@frankier
Copy link
Collaborator

frankier commented Nov 5, 2022

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 SimonDanisch/Bonito.jl#131

@frankier
Copy link
Collaborator Author

frankier commented Nov 5, 2022

@Dsantra92

@Dsantra92
Copy link
Collaborator

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
Copy link
Collaborator

Looks like JSSserve.jl supports HTTP 0.8 and 0.9 only. But according to SimonDanisch/Bonito.jl#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
Copy link
Collaborator Author

frankier commented Nov 5, 2022

Okay. Seems reasonable. Thanks!

@frankier
Copy link
Collaborator Author

frankier commented Nov 5, 2022

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
Copy link
Collaborator

t-bltg commented Nov 26, 2022

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
Copy link
Collaborator

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

@t-bltg
Copy link
Collaborator

t-bltg commented Nov 26, 2022

Compat, can't update to latest DemoCards.

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

@Dsantra92
Copy link
Collaborator

Looks like more of a WebSockets.jl issue here. Track here JuliaWeb/WebSockets.jl#181

@Dsantra92
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

t-bltg commented Nov 27, 2022

What is @johnnychen94's opinion on this matter ?

@johnnychen94
Copy link
Member

johnnychen94 commented Nov 27, 2022

There are two issues to solve here:

  • HTTP 1.0 compatibility. This should be fixed in other packages (e.g., compatible with HTTP [1.1.0 - 1.6.0) JuliaWeb/WebSockets.jl#182). It seems that WebSockets is approaching HTTP > 1.0 compatibility.
  • the using DemoCards time. I agree that probably not all users need it, and it would be a good trade-off to introduce Requires.jl to load it conditionally. But I'm not sure how much code changes are involved here. Yet I do think we need to install it and manage the Pluto dependencies in Project.toml

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

  • revert the pluto-releated changes, tag a new v0.4.12 release
  • tag a v0.5.0 release for Pluto-related features -- this is probably a large feature that requires a new breaking change.
  • and keep v0.4.x until the compatibility issue is solved.

How would you guys think?

@t-bltg
Copy link
Collaborator

t-bltg commented Nov 27, 2022

It is unclear to me why #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 JuliaLang/julia#47040 or JuliaLang/julia#47695, but that is long term.

@hustf
Copy link

hustf commented Nov 29, 2022

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
Copy link
Collaborator

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.

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 a pull request may close this issue.

5 participants