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

Move df macro here? #3336

Open
mkborregaard opened this issue Mar 5, 2021 · 8 comments · May be fixed by #3351
Open

Move df macro here? #3336

mkborregaard opened this issue Mar 5, 2021 · 8 comments · May be fixed by #3351

Comments

@mkborregaard
Copy link
Member

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

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

@mkborregaard
Copy link
Member Author

Notably, this would not be breaking

@daschw
Copy link
Member

daschw commented Mar 14, 2021

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 daschw linked a pull request Mar 14, 2021 that will close this issue
@daschw
Copy link
Member

daschw commented Mar 14, 2021

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):

  • groupedbar
  • (grouped)boxplot
  • andrewsplot
  • corrplot
  • covellipse
  • ecdf

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

@isentropic
Copy link
Member

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

@mkborregaard
Copy link
Member Author

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

piever commented Mar 16, 2021

A bit late to the party, but I also agree with @daschw. Only extra comment is that after JuliaStats/KernelDensity.jl#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
Copy link
Contributor

Maybe rename it to @table macro then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants