scikit-hep
fix: overflow bug
Fix the bug mentioned in #436
Under/overflow bins define as non-zero value
Hi @Ming-Yan, thanks for the quick fix and sorry about the delay in taking a look. Could you double-check this and make sure we test across the matrix of options of the 4 flow parameters and different combinations of possible hist objects?
I am currently getting a shape error with the following MRE
h = hist.new.Reg(10, 0, 1, name='x').Var([0,1,2,3], name='y').Weight()
h.fill(np.random.normal(0.5, 0.5, 1000), np.random.choice([0,1,2,3], 1000))
h.plot(flow='show')
Hi @andrzejnovak , thanks for the feedback, indeed the padding nan axis and bins are not correctly implemented in the previous iteration, now fix in this update.
I also check the failure on the CI pipeline(it's good with my local test with the command(also CI in python 3.8) but still some error for 3.7...
python -m pytest -r sa --mpl --mpl-results-path=pytest_results
Did the existing baseline images change? Perhaps the updated version is missing from the PR?
Hmmm... no, the local test only has problem with lhcb font style...
Local test
CI
I am a bit confused, the diff you linked seems to be for the flow test right? I'd expect the changes not affect the LHCb style here so if it's failing locally it shouldn't be an issue (you're probably missing msft fonts - LHCb uses Times New Roman).
If it'd help, we're gonna drop python 3.7 anyway in #428 so I can merge that first if you'd want to rebase on that.
I am a bit confused, the diff you linked seems to be for the flow test right? I'd expect the changes not affect the LHCb style here so if it's failing locally it shouldn't be an issue (you're probably missing msft fonts - LHCb uses Times New Roman).
Indeed the first plot(white background) is the local test, where no issue with flow plot but missing font in the machine. The second test is from the CI pipeline where the failure of test_hist2dplot_flow appears.
If it'd help, we're gonna drop python 3.7 anyway in #428 so I can merge that first if you'd want to rebase on that.
Yeah, I think this would be helpful🙂