Hist.plot_pull: more suitable bands in the pull bands 1sigma, 2 sigma, etc.

[eduardo-rodrigues]

I was playing with Hist.plot_pull and noticed that the range of the pulls is dynamic, meaning it is calculated from max(np.abs(pulls)) / pp_num in the code. This is not ideal since (1) pull plots are basically always displayed between -5 and 5, and (2) having several pull plots side-by-side would likely provide ranges on a plot-by-plot case, which is not ideal.

This little PR fixes the issue. It also makes sure that the colour bands by default are set at 1/2/.../5 sigma.

I will post an issue with some other matters related to the function.

Next week I will be giving some lectures and will be showing Hist. If at all possible it would be great to have this in a release :-).

[eduardo-rodrigues]

With these changes I get a plot of the kind (using the colours exemplified in the ReadTheDocs)

[andrzejnovak]

Isn't it strange to put in such magic numbers? What if the pulls are very close to 0? Imo it's a reasonably easy to manually set ylims if desired.

[eduardo-rodrigues]

Have you ever seen anyone not plotting pulls between -5,5? Maybe -3,3, but that's about the only reasonable thing to do. There's no magic, just standard usage of pulls (even more relevant when you want to visually compare several ones, in which case you really want to be looking at the same limits).

BTW, I would be more than happy with a configuration of the function. So far one needs to look at the code to see all magic keywords to define to have a nice plot, and all that should have defaults and documentation. But that's another question, which I did not post about for lack of time right now.

I would be happy to later have the ylim configurable. Sure, makes sense. But having calculated as it is now makes no sense, really.

I don't think you can reasonably easy change the ylim manually. Can you? Then you need to make the plot, get hold of the relevant object and replot. No?

[andrzejnovak]

So I'm just armchair-ing this, since I haven't had the opportunity to play with hist much yet, but the method seems to return both axes, so something like this should work

main_ax, pull_ax = hist.pull_plot(...)
pull_ax.set_ylim(-5, 5)

[eduardo-rodrigues]

Me either, I admit. I just started playing with it last night.

Thanks, indeed I had not spotted that and your suggestion works. I would still like to see some of the defaults change, though ...

[henryiii]

I believe it was supposed to return artists, not axes. You can get the axes from an artist; it's much harder the other way around.

It probably would be a good config option. Like plim=3 for -3 to 3 and plim=(-3,4) for -3 to 4?

[eduardo-rodrigues]

OK.

Yes, some options are very useful. And, as said, already, the fact that you typically need to give many kwargs such as

ax, pull_ax = plot_pull(
    hh,
    pdf,
    eb_ecolor="steelblue",
    eb_mfc="steelblue",
    eb_mec="steelblue",
    eb_fmt="o",
    eb_ms=6,
    eb_capsize=1,
    eb_capthick=2,
    eb_alpha=0.8,
    fp_c="hotpink",
    fp_ls="-",
    fp_lw=2,
    fp_alpha=0.8,
    bar_fc="royalblue",
    pp_num=5,
    pp_fc="royalblue",
    pp_alpha=0.5,
    pp_ec=None,
    ub_alpha=0.2,
)

to have a nice plot is far too much, especially that there is no way to know their list apart from the RTD example.

[andrzejnovak]

OK.

Yes, some options are very useful. And, as said, already, the fact that you typically need to give many kwargs such as

ax, pull_ax = plot_pull(
    hh,
    pdf,
    eb_ecolor="steelblue",
    eb_mfc="steelblue",
    eb_mec="steelblue",
    eb_fmt="o",
    eb_ms=6,
    eb_capsize=1,
    eb_capthick=2,
    eb_alpha=0.8,
    fp_c="hotpink",
    fp_ls="-",
    fp_lw=2,
    fp_alpha=0.8,
    bar_fc="royalblue",
    pp_num=5,
    pp_fc="royalblue",
    pp_alpha=0.5,
    pp_ec=None,
    ub_alpha=0.2,
)

to have a nice plot is far too much, especially that there is no way to know their list apart from the RTD example.

Indeed. Exposing the artitst should help with that, so that you don't have to pass everything to the kwargs, which are custom. Anyway I would suggest at least the colors just be tied together unless passed. Not necessarily in this PR.

    eb_ecolor="steelblue", # this should just fetch automatically from cycler
    eb_mfc=None, # fetch the color of the baseline artist if None
    eb_mec=None, # fetch the color of the baseline artist if None

[henryiii]

@all-contributors, please add @eduardo-rodrigues for code.

[henryiii]

@jpivarski This is why I said I found all-contributors's bot to be flaky...

[henryiii]

@eduardo-rodrigues removed the needs changelog label 5 hours ago

I'm pretty sure the bot would have done it once you had an entry in the changelog.

[eduardo-rodrigues]

@eduardo-rodrigues removed the needs changelog label 5 hours ago

I'm pretty sure the bot would have done it once you had an entry in the changelog.

Sorry, I did not notice it was a bot and removed it. My bad, you are using some tools I'm not yet familiar with.

[henryiii]

No problem, I'm just trying them out currently. It worked (it prompted you to add a changelog). :)