Misplaced **hist_kwargs?

[Cosmicstring]

It seems that this line inside core.py contains **hist_kwargs argument, even though it is a matplotlib.pyplot function that is called. Should instead this be passed a few lines above to the np.histogram call, i.e. here? Possibly there could be two different kwargs passed for the np.histogram and the pyplot?

[dfm]

Great question!

While it is calling .plot rather than .hist, that line is plotting the histogram so the way I've always used it (e.g. to set the line styles) is consistent with it as written… but I agree that it's not great: .hist and .plot do take some different arguments, and you could end up with different styles when you turn smoothing on and off. It would be great to have a consistent interface (that's hopefully also backwards compatible), but I don't know the best way to design it!

[Cosmicstring]

I agree, I am not sure what is the best option. Maybe introducing a new dictionary just before plotting and removing any argument not in ax.plot kwarg list?

On a side note, I believe also that it is a bit misleading to make the plotting of the 1D pdfs as if the actual histogram values are plotted when smooth1d is not None. In other words, the code first finds the histogram and then smooths it. If it were the other way around, i.e. if the bins were first smoothed and then on these the histogram was obtained I think it would make sense to plot the final result as a histogram. The way it currently is I think is a bit misleading. It might be better to plot a continuous line instead. What do you think?

What I did was the following:

if smooth1d is None:
   n, _, _ = ax.hist(x, bins=bins_1d, weights=weights, **hist_kwargs)
else:
   if gaussian_filter is None:
       raise ImportError("Please install scipy for smoothing")
   n, _ = np.histogram(x, bins=bins_1d, weights=weights, density=True)
   n = gaussian_filter(n, smooth1d)
   x0 = np.array(list(zip(bins_1d[:-1], bins_1d[1:]))).flatten()
   y0 = np.array(list(zip(n, n))).flatten()
   if smooth1d is not None and interp1d is not None:
       # Now use a continuos line for the plot instead of histogram counts
       bins_1d_centers = 0.5*(bins_1d[1:] + bins_1d[:-1])
       pdfinterpolated = interp1d(bins_1d_centers, n, fill_value="extrapolate")
       newx = np.linspace(bins_1d.min(), bins_1d.max(), 2*n_bins_1d)
       ax.plot(newx, pdfinterpolated(newx), **hist_kwargs)
   else:
       ax.plot(x0, y0, **hist_kwargs)

with a small addition in the header of the core.py for checking whether interp1d is imported properly:

try:
     from scipy.interpolate import interp1d
 except ImportError:
     interp1d = None

This produces

instead of

Let me know how this looks to you.

[Cosmicstring]

@dfm any thoughts?

[dfm]

I think this is a great idea, but I've been a little slow to respond because I'm waffling on API design. But, I think it would be good to implement this regardless. Want to open a PR switching to this?