-
Notifications
You must be signed in to change notification settings - Fork 13
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
Replace PyPlot calls with Plots.jl #66
Comments
I think rewriting to Plots.jl is a good idea, partly because right now the following does not work:
This is because the last command will initialize PyPlot.jl, which searches for an existing installation of matplotlib, and if not found, it prints an error with instructions. About 1.2.3.: If you use Plots.jl (1.), then you can write the plot functions without adding Plots.jl as a dependency to ClimateMARGO, so (2.) and (3.) are no longer necessary. There is a package specifically for this situation: https://github.com/JuliaPlots/RecipesBase.jl . It is a "mock version" of Plots.jl that allows packages to define their own plots using |
Will this be transparent enough for users? Like it will give instructions when you try to plot? I don’t want people to expect plots and not know why they don’t show up. |
That's a good point -- currently it does not, but I have submitted this to JuliaPlots/Plots.jl#4431 If they can't fix it soon, we can write our own workaround to print this warning. |
We can also just make Plots.jl one of our dependencies (without the Recipes trick). Maybe we are over-optimizing by not including it, even though users of ClimateMARGO will very likely want to plot the results. |
Addressing this issue in #84 |
@fonsp has mentioned that having PyPlot as a dependency for ClimateMARGO.jl unecessarily complicates things.
I see three options for moving forward:
What do you think @fonsp? My understanding is that you already have a hacky way of removing the plotting package dependencies– is this even necessary? I think it might be desirable even for your average user because it will decrease build / binder spin-up time?
The text was updated successfully, but these errors were encountered: