Plots.jl icon indicating copy to clipboard operation
Plots.jl copied to clipboard

Plots 2.0

Open BeastyBlacksmith opened this issue 1 year ago • 31 comments

This is the development branch for Plots 2.0.

What changed

  • No default backend in PlotsBase, Plots becomes a wrapper for PlotsBase + GR (per https://github.com/JuliaPlots/Plots.jl/issues/4565#issuecomment-2004679230), making it non-breaking;
  • Backends are purely extensions with no @require;
  • The internals are more organized into modules and moved around;
  • Backend init and backend code is only found in its extensions, this should help with backend interference and annoyances about initialization and precompilation of plots. This should make backends modular and implemented much easier and without hacks into Plots.jl codebase;
  • Restored Preferences chosen backend;

Example:

import UnicodePlots
using Plots 
unicodeplots()
plot(1:10)

What works

  • PlotsBase and Plots tests;
  • Preferences based backend selection.
  • Precompilation statements for Plots, using an alternate backend

What does not work

Help needed:

  • [x] write backend agnostic precompile statements for PlotsBase
  • [x] backport changes from v1 since last rebase
  • [ ] report breakages
  • [x] move documentation back into this repo
  • [x] start updating the docs
  • [ ] update the news.md
  • [ ] register GraphRecipes and StatsPlots from this monorepo to the General registry.

Decisions to be made

  • [x] deprecate inspectDR ?
  • [ ] move to column major order ?
  • [ ] change the defaults

Issues needing attention please add more or checkoff resolved: https://github.com/JuliaPlots/Plots.jl/issues/4565#issue-1466819814 Copy of that: Labeled issues:

  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/4532
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/4028
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/3001
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/2980
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/2129
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/2047
  • [x] https://github.com/JuliaPlots/Plots.jl/issues/4181
  • [x] https://github.com/JuliaPlots/Plots.jl/issues/4513
  • [x] https://github.com/JuliaPlots/Plots.jl/issues/4631
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/4350
  • [x] https://github.com/JuliaPlots/Plots.jl/issues/4567
  • [ ] https://github.com/JuliaPlots/Plots.jl/issues/4596

Labeled PRs:

  • [x] https://github.com/JuliaPlots/Plots.jl/pull/4326
  • [ ] https://github.com/JuliaPlots/Plots.jl/pull/3942
  • [x] https://github.com/JuliaPlots/Plots.jl/pull/3744
  • [x] https://github.com/JuliaPlots/Plots.jl/pull/3732
  • [ ] https://github.com/JuliaPlots/Plots.jl/pull/3473
  • [ ] https://github.com/JuliaPlots/Plots.jl/pull/3357
  • [ ] https://github.com/JuliaPlots/Plots.jl/pull/3351
  • [x] Rework adding legends #3732

Closes #4326

BeastyBlacksmith avatar Mar 12 '24 14:03 BeastyBlacksmith

@t-bltg do you intend to release this as a 1.x release?

BeastyBlacksmith avatar Apr 02 '24 07:04 BeastyBlacksmith

If we can make it non breaking, it would make things easier and end endless discussions.

t-bltg avatar Apr 02 '24 07:04 t-bltg

Well, there would be a few changes that we'd need to roll back for this, like removal of pgfplots and I removed some functions also.

But I also don't quite see how to make switching of backends non-beraking since, what was

using Plots
pythonplot()

is now

using Plots
import PythonPlot
pythonplot()

BeastyBlacksmith avatar Apr 02 '24 08:04 BeastyBlacksmith

I still don't get why can't 2.0 be breaking and why having a helpful error message isn't enough as a solution as per my comment in the issue #4565. Having GR as the default backend feels bad to me. I understand that nobody changes the backend but this way backend malfunctioning could be diagnosed easier.

Then if the backend doesn't work people can learn about other backends. I really don't buy the keep gr argument by searching GitHub code. Plots 2.0 isn't just for the users but also for debugging and long term stability.

Keeping gr as the default is not good imo. Flux got rid of it's CUDA for this reason and it works fine, new users can adapt we just need to leave good info/warn messages

isentropic avatar Apr 02 '24 17:04 isentropic

I'm sorry that I'm late into the discussion and if some undoing needs to be done but we should prioritise long term good with 2.0. GR being the default is also what defers people from using plots, if users can learn about the flexibility that's great too. Pyplot imo works more stably with various fonts and scalings.

The purpose of package extensions is exactly this. Splitting into plotsbase and just plots to keep the standard interface means that we never need to go to 2.0 then.

2.0 by definition is breaking and that's ok. Plots is not a low level package and hardly any packages take dep on Plots. Interactive usage is easier to adapt to 2.0 anyways in case the help messages are well stated.

We also need to think about column major order to make Plots play nicely with others and streamline the column major way. These two changes are by far the most important, both being breaking and that's ok cause it's 2.0.

isentropic avatar Apr 02 '24 17:04 isentropic

I get why people might object with needing to import GR explicitly, but you know adding such import help long term in that people would realize that there are more backends than just GR, and that the errors are coming because of 'import GR' not 'using Plots'. Im not kidding half of our issues are because GR didn't build or something like that. In the current scenario users try to import Plots it errors and then there's frustration. With gr free plots GR will throw it's warnings right awa and we can throw warnings and stuff like your GR loading failed, try these other backends.

isentropic avatar Apr 02 '24 17:04 isentropic

But I also don't quite see how to make switching of backends non-beraking since, what was

I've omitted this, so I've restored the 2.0 label.

We also need to think about column major order to make Plots play nicely with others and streamline the column major way.

I get this, but if no one implements it, this shouldn't block a release.

Having GR as the default backend feels bad to me.

I would be in favor of dropping GR as the default backend. But on the other end, adding PlotsBase without any backend dependency seems a good compromise.

In https://github.com/JuliaPlots/Plots.jl/pull/4914, I've finalized the way modules and extensions should be written with a lot of cleanups, and restoration of the precompile statements, but unfortunately, it broke the reference tests in subtle ways. What a mess !

I like the new splitting of PlotsBase into smaller modules initiated in this branch, but it needs more work and cleanup, without breaking the tests.

t-bltg avatar Apr 06 '24 10:04 t-bltg

I've made progress and fixed the tests in https://github.com/JuliaPlots/Plots.jl/pull/4914, everything is passing locally, ~~and partially in CI~~.

Currently, there is no solution to generate precompile statements for a different backend than GR in Plots since we cannot @eval Main import UnicodePlots for example.

~~So we need at least to generate backend independent precompile statement for PlotsBase.~~ ==> EDIT: done in https://github.com/JuliaPlots/Plots.jl/pull/4914.

t-bltg avatar Apr 07 '24 07:04 t-bltg

@BeastyBlacksmith, how can we move forward now ?

Embedded modules need more cleanup, but at least the port to extensions is now complete, with 100% previous tests passing.

t-bltg avatar Apr 07 '24 13:04 t-bltg

I'd say we merge this now in master, create a v1 branch for backports and move on.

We can probably ask users to try an alpha version of Plots 2.

EDIT: we can also rename the master branch to main.

t-bltg avatar Apr 09 '24 20:04 t-bltg

@BeastyBlacksmith, how can we move forward now ?

Embedded modules need more cleanup, but at least the port to extensions is now complete, with 100% previous tests passing.

Thanks a lot! Next steps would involve porting all the 2.0 labeled PRs and merge them into this branch. I won't merge this branch into master prior to the meeting at JuliaCon and even after that, there is no need to rush this. We should make sure we get all the breaking changes in that have accumulated over the years.

There will be a good amount of time before we get that oppurrtunity again.

BeastyBlacksmith avatar Apr 15 '24 11:04 BeastyBlacksmith

Thanks, please keep in mind that I probably won't work on this anymore since my time is very limited, so make sure to backport the changes made in master into this branch, so that it doesn't go out of sync and stalls.

CI is currently a bit hacked for GraphRecipes and StatsPlots which will need to adapt to these changes (likely trivial changes).

Also, please make sure the test suite remains green, this took quite some time ...

t-bltg avatar Apr 15 '24 12:04 t-bltg

Don't you think that making v2 the current master/main would force developers to work towards the next version (and thus force backporting to v1) ?

I mean we can always merge PRs into a v1 branch and tag backports releases.

Else I'm afraid, given the amount of active contributors, that this would be a nightmare to maintain in sync between branches ...

t-bltg avatar Apr 16 '24 09:04 t-bltg

What I probably will do is sync them and then make v2 the new default branch, possibly renaming master to v1. I don't think we need to merge these at all. We could also make it a policy, to only merge stuff to v1 if it has a v2 port.

BeastyBlacksmith avatar Apr 16 '24 10:04 BeastyBlacksmith

nightly and 1.11.0-beta1 are passing :tada:, it has been a while ...

t-bltg avatar Apr 18 '24 07:04 t-bltg

Codecov Report

Attention: Patch coverage is 89.45247% with 314 lines in your changes are missing coverage. Please review.

Project coverage is 87.06%. Comparing base (fa65e7d) to head (0ab074b). Report is 16 commits behind head on master.

:exclamation: Current head 0ab074b differs from pull request most recent head f32247a

Please upload reports for the commit f32247a to get more accurate results.

Files Patch % Lines
PlotsBase/src/Commons/attrs.jl 75.28% 86 Missing :warning:
PlotsBase/src/utils.jl 92.44% 37 Missing :warning:
PlotsBase/src/Annotations.jl 75.86% 28 Missing :warning:
PlotsBase/src/backends.jl 86.25% 18 Missing :warning:
PlotsBase/src/axes_utils.jl 91.80% 15 Missing :warning:
PlotsBase/src/plotly.jl 82.50% 14 Missing :warning:
PlotsBase/src/Axes.jl 93.95% 13 Missing :warning:
PlotsBase/src/Plots.jl 89.60% 13 Missing :warning:
PlotsBase/src/recipes.jl 87.62% 12 Missing :warning:
PlotsBase/src/Commons/Commons.jl 92.36% 10 Missing :warning:
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4904      +/-   ##
==========================================
- Coverage   89.77%   87.06%   -2.71%     
==========================================
  Files          40       51      +11     
  Lines        8780     5993    -2787     
==========================================
- Hits         7882     5218    -2664     
+ Misses        898      775     -123     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 18 '24 08:04 codecov[bot]

GraphRecipes and StatsPlots are now integrated into Plots repo, so no more commits should be merged in https://github.com/JuliaPlots/GraphRecipes.jl or https://github.com/JuliaPlots/StatsPlots.jl.

t-bltg avatar Oct 13 '24 18:10 t-bltg

@BeastyBlacksmith, can you fix the DOCUMENTER_KEY issue so that we can have dev docs for v2 deployed ?

t-bltg avatar Oct 21 '24 18:10 t-bltg

I've changed the deploydocs url to https://github.com/JuliaPlots/Plots.jl.git in https://github.com/JuliaPlots/Plots.jl/commit/a6562c14dc4e9f23b58958807f1afd19073dc8dd specifically because I was hitting a deployment failure from Documenter.

So I think restoring it in https://github.com/JuliaPlots/Plots.jl/pull/4904/commits/912824f7011f071303c57280186bd7e688044e91 will hit this error again.

I thought we were dropping the PlotDocs repo ?

t-bltg avatar Oct 24 '24 13:10 t-bltg

I thought we were dropping the PlotDocs repo ?

Not entirely, it should still host the artifacts to not blow the repo size of Plots, but the generating code should be here.

BeastyBlacksmith avatar Oct 24 '24 13:10 BeastyBlacksmith

I see.

But still, we will hit this non-deployment criterion because of the url: https://github.com/JuliaPlots/Plots.jl/actions/runs/11317817828/job/31471690944#step:5:7953 │ - ✘ ENV["GITHUB_REPOSITORY"]="JuliaPlots/Plots.jl" occurs in repo="github.com/JuliaPlots/PlotDocs.jl.git"

After changing it in https://github.com/JuliaPlots/Plots.jl/commit/a6562c14dc4e9f23b58958807f1afd19073dc8dd , we hit another error, related to DOCUMENTER_KEY: https://github.com/JuliaPlots/Plots.jl/actions/runs/11318156624/job/31484813837#step:5:7945 │ - ✔ ENV["GITHUB_REPOSITORY"]="JuliaPlots/Plots.jl" occurs in repo="github.com/JuliaPlots/Plots.jl.git"

, which was my original request in https://github.com/JuliaPlots/Plots.jl/pull/4904#issuecomment-2427473266.

t-bltg avatar Oct 24 '24 14:10 t-bltg

I think we need to follow https://documenter.juliadocs.org/stable/man/hosting/#Out-of-repo-deployment.

Can you do "3. Add the DOCUMENTER_KEY secret to the "source" repository (that runs the documentation workflow)", since I do not have enough rights ?

t-bltg avatar Oct 24 '24 14:10 t-bltg

That key is already present. We used to trigger the build from here in v1 also. You just can't deploy to this repository with it.

BeastyBlacksmith avatar Oct 24 '24 14:10 BeastyBlacksmith

Item 4. done in https://github.com/JuliaPlots/Plots.jl/pull/4904/commits/bb14fd5d5278001e63d035c91951a6c27b1a2cf0, let's see how it goes ...

t-bltg avatar Oct 24 '24 14:10 t-bltg

As I suspected, there is still an issue with permissions, hence I think DOCUMENTER_KEY is the culprit here, but I have no further means to investigate: https://github.com/JuliaPlots/Plots.jl/actions/runs/11501098562/job/32012907078?pr=4904#step:5:8035.

t-bltg avatar Oct 24 '24 15:10 t-bltg

Works now

BeastyBlacksmith avatar Oct 26 '24 19:10 BeastyBlacksmith

Awesome thanks, https://docs.juliaplots.org/dev looks functional !

EDIT: except for the backend examples e.g. https://docs.juliaplots.org/dev/gallery/gr/@ref%20gr_ref001 is 404, when clicking on the thumbnails.

t-bltg avatar Oct 26 '24 19:10 t-bltg

The docs are now fully functional on v2, e.g. https://docs.juliaplots.org/dev/gallery/gr/generated/gr-ref002/#gr_ref002.

t-bltg avatar Nov 02 '24 13:11 t-bltg

@BeastyBlacksmith, would you be in favor of integration PlotThemes into this monorepo ?

t-bltg avatar Nov 02 '24 16:11 t-bltg

The docs are now fully functional on v2,

Thanks a lot !

would you be in favor of integration PlotThemes into this monorepo ?

Yeah, thats probably a good idea

BeastyBlacksmith avatar Nov 04 '24 09:11 BeastyBlacksmith