AverageShiftedHistograms.jl
AverageShiftedHistograms.jl copied to clipboard
Removing UnicodePlots as a dependency to reduce package load time?
I'd love to use AverageShiftedHistograms for density estimation in BAT.jl (density estimation for marginals) and other places, but the dependency on UnicodePlots makes AverageShiftedHistograms very heavy. UnicodePlots is a monster with a load time of about 3 seconds, whereas AverageShiftedHistograms itself is very lightweight, and it's other dependencies are not very heavy either.
@joshday how would you feel about removing UnicodePlots as a dependency? Users would have to call plot explicitly then, of course, but it would likely reduce the load time of AverageShiftedHistograms drastically.
Yeah, I think I'm okay with that, although I do like the instant feedback of the density in the show methods.
Would supporting DensityInterface be sufficient for your use case? See https://github.com/joshday/AverageShiftedHistograms.jl/pull/40
Thanks Josh - yes, I was actually about to suggest supporting DensityInterface, I'd overlooked #40 :-)
DensityInterface is very lightweight by design, and it's densityof and logdensityof can replace the custom PDF-functions in AverageShiftedHistograms. I think we should have DensityKind(::Ash) = HasDensity(), since an Ash is semantically actually a probability measure/distribution.
DensityInterface currently doesn't offer a CDF-Function, but we may have a lightweigt DistributionsBase in the future (JuliaStats/Distributions.jl#1139).
I can do a PR for the DensityInterface support if you like?
That would be great! I'm traveling this week so my availability is limited.
No worries!
@oschulz @joshday still interested in this? Would make plotting a lot easier with ASH.
Sure. You had a PR but it needed some changes. Feel free to reopen it
Oh, that's why! I could've sworn I'd made a PR for this at some point. Can you reopen it? I don't have permission.
I'm definitely still interested.
Reopened!
+1 for this. On Julia v1.10.0-beta1, this package for me takes 0.66s to load with UnicodePlots and 0.16s without UnicodePlots. As a dependency, that 0.16s is probably nothing, since most packages that would add this as a dep probably also (in)directly depend on the rest of ASH's dependencies.
You could always do something like this
const SHOW_UNICODE_PLOTS = Ref(false)
function show_unicode_plots(tf::Bool)
SHOW_UNICODE_PLOTS[] = tf
end
function Base.show(io::IO, ::MIME"text/plain", o::Ash)
...
if SHOW_UNICODE_PLOTS[]
_show_unicode_plot(io, o)
end
end
function _show_unicode_plot end
And then in an AverageShiftedHistogramsUnicodePlotsExt extension overload
AverageShiftedHistograms._show_unicode_plot(io::IO, o::Ash) = ...
Then if a user really wants the nice REPL output, they can load UnicodePlots themselves and enable it manually.
@sethaxen I'd be cool with a package extensions implementation. I won't be able to work on it for a while, but I would accept a PR.