scikit-hep / mplhep

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

feat: top right corner position, inside the frame, with loc=5 parameter #423

Open asantra opened 1 year ago

asantra commented 1 year ago

Addition of loc parameter value 5. If loc=5, the experiment label and lumi text are plotted on the top right-hand side, inside the plot frame.

andrzejnovak commented 1 year ago

Welcome @asantra. Can you add it to the test https://github.com/scikit-hep/mplhep/blob/master/tests/test_styles.py#L216 ? Maybe reshape the grid as 2x3 instead of 1x5.

I am a bit curious how the alignment will look like.

asantra commented 1 year ago

Hi @andrzejnovak I have added the new location in the tests. Please have a look and let me know if anything needs to be changed.

Best, Arka

andrzejnovak commented 1 year ago

Please add the updated test image as well CONTRIBUTING.md (don't add the unchanged images, git likes to flag them as different)

asantra commented 1 year ago

Hi @andrzejnovak , Here is the updated test image: https://github.com/asantra/mplhep/blob/arka-dev/tests/baseline/test_label_loc.png

The new style is shown in the sixth frame (bottom right).

Best, Arka

andrzejnovak commented 1 year ago

Sorry about the broken GHA reporting, a fix should already be on the way https://github.com/pytest-dev/pytest-github-actions-annotate-failures/pull/70. Checking the repo out locally it seems there are font differences. If you run pytest locally on master, do you get passing tests?

asantra commented 1 year ago

Hi @andrzejnovak , I tested the tests/test_styles.py on the master branch and this is the result:

pytest tests/test_styles.py

======================================================================test session starts ======================================================================= platform darwin -- Python 3.9.7, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 Matplotlib: 3.4.3 Freetype: 2.10.4 rootdir: /Users/arkasantra/HepProjects/myEdits/mplhep plugins: anyio-2.2.0, mock-3.10.0, mpl-0.16.1, check-2.1.4 collected 36 items

tests/test_styles.py sssss..sssssssssssssssssssssssssssss [100%]

================================================================= 2 passed, 34 skipped in 1.61s ==================================================================

I think that most of the tests are skipped because I am not using a linux system.
How should I test only the relevant part (i.e. test_label_loc() function in test_styles.py)?

Best, Arka

andrzejnovak commented 1 year ago

What about the test tests in test_basic.py? We currently strip most text for the comparisons, but some of them have inherent bin labels such as https://github.com/scikit-hep/mplhep/blob/master/tests/baseline/test_hist2dplot.png.

Note if you ran with the generate option, you might already have new "reference" files produced with the local font, so the tests won't fail locally.

You can run a specific function in pytest with pytest tests/test_styles.py::test_label_loc()

asantra commented 1 year ago

Hi @andrzejnovak , The test_basic.py fails on my system. Here is the short summary:

==================================================================== short test summary info ===================================================================== FAILED tests/test_basic.py::test_histplot_flow - AttributeError: 'StepPatch' object has no property 'flow' FAILED tests/test_basic.py::test_histplot_hist_flow - AttributeError: 'StepPatch' object has no property 'flow' FAILED tests/test_basic.py::test_histplot_uproot_flow - AttributeError: 'StepPatch' object has no property 'flow' FAILED tests/test_basic.py::test_histplot_type_flow - AttributeError: 'StepPatch' object has no property 'flow' FAILED tests/test_basic.py::test_hist2dplot_flow - AttributeError: 'QuadMesh' object has no property 'flow' ================================================================== 5 failed, 46 passed in 8.37s ==================================================================

Here is the full output: https://docs.google.com/document/d/1wXfWkzklqpR1g-5-pm2aje-fpW67xDdmhV0fjyiWY5k/edit?usp=sharing

It seems like some software mismatch in my system.

Best, Arka

andrzejnovak commented 1 year ago

The errors you're getting now are about a recently merged PR, can you rebase on master and try again?

asantra commented 1 year ago

Hi @andrzejnovak , I am sorry; I did not understand your instruction. I cloned the master branch from scikit-hep/mplhep in a separate area. I put my edited files in that new area. Then I did the pytest: pytest tests/test_basic.py

. Still, I am getting the same set of errors are before: https://docs.google.com/document/d/1wXfWkzklqpR1g-5-pm2aje-fpW67xDdmhV0fjyiWY5k/edit?usp=sharing

I will appreciate if you help me out here. May I ask for the exact git commands that I need to follow?

Best, Arka

jonas-eschle commented 7 months ago

Hi @asantra , apologies for the delay: can you locally rebase on master (or merge?). Do you need any help with that?

If you've done that, can you push, so that we see all the changes in the PR