search

JuliaTeX / PGFPlots.jl

This library uses the LaTeX package pgfplots to produce plots.
Other
188 stars 36 forks source link

Lazy-loading DataFrames.jl reduces the package loading time #154

Closed mirkobunse closed 4 years ago

mirkobunse commented 4 years ago

This PR uses Requires.jl to load DataFrames.jl only when actually needed. It reduces the output of @time using PGFPlots from 19s to 16s, on my machine. The script which makes the following time measurements is given in issue #130.

Trial 0/3: 28.656163 seconds (30.13 M allocations: 1.544 GiB, 1.53% gc time)
Trial 1/3: 15.671989 seconds (27.89 M allocations: 1.435 GiB, 2.67% gc time)
Trial 2/3: 15.979555 seconds (27.89 M allocations: 1.435 GiB, 2.62% gc time)
Trial 3/3: 16.170125 seconds (27.89 M allocations: 1.435 GiB, 2.62% gc time)
Mean @time (ignoring the first output due to possibly required pre-compilation):
 15.940556333333333 seconds (27.89 M allocations: 1.4349999999999998 GiB, 2.6366666666666667% gc time)

Lazy-loading DataFrames.jl also makes perfect sense from a design perspective because the dependency is only needed to implement wrappers around the actual functions. Users only need these wrappers if they have loaded DataFrames, already.

mykelk commented 4 years ago

It is strange that it is failing on ERROR: LoadError: LoadError: UndefVarError: hasproperty not defined in Julia 1.0 on Travis, but it seems to have passed in @mossr 's most recent commit. I'm probably missing something?

mossr commented 4 years ago

Odd...hasproperty came from Compat when using NBInclude but something I'm also missing is going on. Regardless, I removed the dependency on hasproperty in master.

mykelk commented 4 years ago

Thanks @mossr . @mirkobunse do you think you can pull in his most recent change? Then we can get this merged. Thank you for your contribution!

mirkobunse commented 4 years ago

It worked. Thanks, @mossr !

mossr commented 4 years ago

@mirkobunse You're very welcome! Thanks for the contribution! I'll merge this now.