JuliaPlots / Plots.jl

Powerful convenience for Julia visualizations and data analysis
https://docs.juliaplots.org
Other
1.84k stars 355 forks source link

Move df macro here? #3336

Open mkborregaard opened 3 years ago

mkborregaard commented 3 years ago

I think this has been discussed several times on StatsPlots, but let's move it here. My suggestion is to simply move the @df macro from StatsPlots to here. Having data in dataframes is a common use case for plotting, and the placement on the macro in StatsPlots is a legacy from julia 0.5 and before, where DataFrames was a massive dependency. With the Tables interface this issue is resolved. @daschw @piever

isentropic commented 3 years ago

i think this is fine, as i believe tables Api is lightweight. I agree that dataframes are quite common

mkborregaard commented 3 years ago

Notably, this would not be breaking

daschw commented 3 years ago

Don't we need it to be breaking? Otherwise, the current StatsPlots release would be compatible with a new Version of Plots that exports the @df macro. I am not sure, how critical this is, if we provide an update to StatsPlots right away, but I think a breaking release would be cleaner.

daschw commented 3 years ago

I think it would in general be nice to move stuff from StatsPlots that does not require an additional dependency in Plots. Most notably, I am sometimes annoyed to require StatsPlots for groupedbar. From a first skimming through StatsPlots code the following could be moved to Plots without an extra dependency (Plots already has StatsBase):

I think groupedbar would be nice to have in Plots and maybe also boxplot, not sure about the others.

isentropic commented 3 years ago

I usually find the need in ecdf, but never understood if it is a really complex recipe to be included in Plots

mkborregaard commented 3 years ago

Oh it might make sense with a breaking release because of StatsPlots - what I meant is that it should not break any user's workflow. Agree on groupedbar and boxplot, and for the others maybe they do belong (and are best documented) in StatsPlots - so essentially I agree with you Daniel.

piever commented 3 years ago

A bit late to the party, but I also agree with @daschw. Only extra comment is that after https://github.com/JuliaStats/KernelDensity.jl/pull/84 loading time of KernelDensity is a bit better, so maybe it is not out of the question to port recipes that rely on it (in practice, (grouped)violin and density). I'm mainly mentioning it because IMO it'd be a bit odd to have boxplot and violin in separate packages (same for histogram and density).

For reference, on my machine and julia 1.6

julia> @time using Plots
  4.435312 seconds (6.89 M allocations: 498.039 MiB, 4.78% gc time, 0.78% compilation time)

julia> @time using KernelDensity
  1.358977 seconds (2.20 M allocations: 147.740 MiB, 8.93% gc time)

julia> @time using Distributions # this one would be for free as it is a KernelDensity dep, but maybe distribution recipes belong in StatsPlots
  0.000669 seconds (251 allocations: 16.328 KiB)
matthieugomez commented 3 years ago

Maybe use this change to think of a better name, e.g. @table?