scikit-hep / mplhep

Extended histogram plotting on top of matplotlib and HEP collaboration compatible styling
https://mplhep.readthedocs.io
MIT License
185 stars 64 forks source link

feat: new "band" histtype #472

Closed andrzejnovak closed 7 months ago

andrzejnovak commented 7 months ago

Adding an error "band" plot style. I am envisioning usage like this

h = hist.new.Reg(10, 0, 1, flow=False).Weight().fill(np.random.normal(0.5, 0.3, 1000))
hep.histplot(h)
hep.histplot(h, histtype='band')

image

What do you think of the defaults @alexander-held @matthewfeickert @iasonkrom ?

ikrommyd commented 7 months ago

I would probably just change the default to just hatching or maybe with some faint background as well instead of this orange color. That with the ability to customize the colors of the errorband should be good.

alexander-held commented 7 months ago

Having bands available as a style is great! First reaction: the additional error bar feels superfluous and possibly confusing, but this is only because you're plotting twice, right? I agree with @iasonkrom that a perhaps slightly darker gray and no additional color may make this more suitable for a wider range of plots by default as the orange might clash, but if this can be configured then it is not a big issue.

andrzejnovak commented 7 months ago

Thanks a lot for the quick feedback both! Indeed the "blue" error comes from the baseline plot. The "band" style only adds well.. the band. The orange is just automatically picked up from the color cycler. Do you think a better default then would be

{
'facecolor:'none', 
'edgecolor': 'darkgrey'
}

?

This might surprise people if they plot more hists at once, but I could see it as a default also.

ikrommyd commented 7 months ago

Well the actual colors like orange sem more surprising to me. That seems better and maybe some very light grey facecolor could also help but in general it should be fine for the users with the ability to customize them.

andrzejnovak commented 7 months ago

The color is just automatic from mpl. If you set a different cycler it will change too. Users can ofc overwrite this by passing facecolor and edgecolor like with any mpl function.

andrzejnovak commented 7 months ago

Updated default hep.histplot(h, histtype='band')

image

ikrommyd commented 7 months ago

Shouldn't there also be a line by default that shows actaully shows the bins heights. I was only talking about the error in my previous comments. Plotting the actual bin heights is good no?

ikrommyd commented 7 months ago

Unless you are thinking of this as an alternative to "errorbar". I do usually think of erorband as a way to plot errors for the a "step" or "fill" histogram.

ikrommyd commented 7 months ago

I guess someone can overlay a "step" histogram with yerr=0 with this and it's fine. Your library, your choice.

andrzejnovak commented 7 months ago

Shouldn't there also be a line by default that shows actaully shows the bins heights. I was only talking about the error in my previous comments. Plotting the actual bin heights is good no?

This would make the implementation more difficult, the line and the band would need to be separate artists, which also makes it finicky for editing the objects after. I see the benefit in syncing the color for example user would run

1) hep.histplot(h, color='green', histtype='band') and the band could plot by default with like alpha=0.5

The alternative case and what I was thinking is 2)

hep.histplot(h, yerr=False, color=c)
hep.histplot(h, histtype='band', alpha=0.5, color=c)

which is more verbose, but it leaves the user with more control.

mplhep does a pass-through of most arguments directly to mpl which is super convenient because we don't have reimplement all possible mpl kwargs. This would bring an issue when the plotted object is composed of two artists that might not have the same settings. Suppose you want a green line with a semitransparent green band. How do we pass the alpha then? If only to the band, then we make it impossible for the user to set the alpha of the line.

andrzejnovak commented 7 months ago

Do we need two methods?

ikrommyd commented 7 months ago
hep.histplot(h, yerr=False, color=c)
hep.histplot(h, histtype='band', alpha=0.5, color=c)

looks good. I would be totally fine as user

andrzejnovak commented 7 months ago

Alright, let's mint this and see what happens :)

alexander-held commented 7 months ago

No strong opinions here but I find it odd having to call two functions to accomplish this. A version

hep.histplot(h, color='green', histtype='band')

which would turn off the errorbar and replace it by the band would feel more natural to me. Maybe it is also the naming that I find confusing,

hep.histplot(h, histtype='band', alpha=0.5, color=c)

to me sounds like "draw the histogram as a band" and does not really imply anything about the thing being drawn actually being a band for the errors. I would expect some kind of hep.errorband(h) or similar to distinguish by name what this actually does.

andrzejnovak commented 7 months ago

I am open to discussing the API. IIUC you suggest that we instead have a new plotting function called errorband which plots the band only aka what currently is histplot(..., histtype='band') and instead the new histype in histplot is called sth like errorband and calls this function with some predefined opts. Like color=nominal_artist.get_color(), alpha=0.5) ?

I think that doesn't get us out of the issue of how to distinguish which kwargs should go to which plotting function when user calls this. alpha is one problem, another is edgecolor which determines both the hatch color as well as the primary line that would be drawn for the nominal.

Re naming I think kwargs should be erring on the side of concise (though hist.plot_ratio is too much) and "band" feels like a reasonable shorthand for "errorband".

I could also see taking advantage of both and provide "band" as is to allow for full user customization and add "errorband" which will be same as "errorbar" with yerr=False and a color-matched but semi-transparent band. We return both artists and hope for the best.