search

HEPData / hepdata_lib

Library for getting your data into HEPData
https://hepdata-lib.readthedocs.io
MIT License
15 stars 39 forks source link

Handle nested TPads to get TObject #176

Closed longjon929 closed 2 years ago

longjon929 commented 3 years ago

I've added another attempt at retrieving the TObject from nested TPads if the path fails. This is useful in the case of a TCanvas with TPads for ratio panels etc.

AndreasAlbert commented 3 years ago

Thanks for the feature implementation! Would you be able to also add a few simple tests? (like e.g. here)

longjon929 commented 3 years ago

Hi @AndreasAlbert , sure. I've tried copying a similar function and modifying it to test this case. It does print out a few lines like this "Cannot find any object in file Name: tmp_7R6HNP4ITWDS.root Title: with path canvas/pad1/testhist. Will try nested TPads .Cannot find any object in file Name: tmp_2TMNFIAEHX4Z.root Title: with path Some/Nonsense/Path. Will try nested TPads" because the first try no longer raises and error. If you would prefer, I can change it to only print something if both attempts fail.

AndreasAlbert commented 3 years ago

thanks! yes, indeed, it would be optimal to only print warnings in case all strategies fail.

longjon929 commented 3 years ago

Ok, I've tidied up the messages.

clelange commented 3 years ago

Hi @longjon929 - thanks for addressing the additional comments! The code looks good to me. Our automated tests, which also include running pylint show some code formatting issues. You can see the details in the check details, I'm also adding them below:

hepdata_lib/__init__.py:229:8: C0200: Consider using enumerate instead of iterating with range and len (consider-using-enumerate)
hepdata_lib/__init__.py:417:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
hepdata_lib/__init__.py:422:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
hepdata_lib/__init__.py:512:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
hepdata_lib/__init__.py:541:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
************* Module hepdata_lib.c_file_reader
hepdata_lib/c_file_reader.py:38:30: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
************* Module hepdata_lib.root_utils
hepdata_lib/root_utils.py:104:0: C0303: Trailing whitespace (trailing-whitespace)
hepdata_lib/root_utils.py:109:0: C0301: Line too long (146/100) (line-too-long)
hepdata_lib/root_utils.py:113:0: C0301: Line too long (146/100) (line-too-long)
hepdata_lib/root_utils.py:95:8: W0612: Unused variable 'err' (unused-variable)

Let us know if you have troubles addressing them and we can help you.

longjon929 commented 3 years ago

Hi @clelange, I've pushed a commit that should fix the warnings in root_utils.py. Since I didn't modify the other files, I didn't look into the other linting warnings.

clelange commented 2 years ago

Sorry, this somehow got lost. I've rebased onto the current master branch so that we get some tests to run.

longjon929 commented 2 years ago

Things look good to me, tests are passing, also the new ones make sense and pass.

Thanks a lot, @longjon929 !

Great, thanks for taking care of the last bits!