cms-tau-pog / TauFW

Analysis framework for tau analysis at CMS using NanoAOD
9 stars 40 forks source link

Make `python3` compatible #36

Closed IzaakWN closed 1 year ago

IzaakWN commented 1 year ago

Make .py files compatible with both python2 and python3. This includes at least the python files that are compiled by scram b. Note that files in $CMSSW_BASE/src/TauFW/*/python need to be python3 compatible for CMSSW_12_X to compile.

This PR addresses Issue https://github.com/cms-tau-pog/TauFW/issues/6, so we can more easily use coffea and correctionlib, as well as become compatible with CMSSW_12_X, where python3 compatibility is required when compiling with scram b.

Any feedback, @smonig, @cardinia?

Changes

As expected, most changes are related to

String type

In some files where basestring is used to capture both str and unicode objects in python2, one needs

from past.builtins import basestring # for python2 compatibility

as basestring is not standard in python3, which does not distinguish str and unicode, see https://python-future.org/compatible_idioms.html#basestring http://docs.buildbot.net/0.9.3/developer/py3-compat.html

Piping

For some code in Plotter/python/corrections/JetToTauFR/, the following

print >> sys.stderr, message

had to be changed to

from __future__ import print_function # for python3 compatibility
print(message,file=sys.stderr)

(As a side note @kchrisucy: JetToTauFR/ should probably be moved to Plotter/. It's quite a large workspace and will be compiled each time a user uses scram b. Is your latest setup different from the current version in the master branch?)

Validation

So far I have only checked that it compiles with scam b in

I still need to run the basic test scripts to ensure nothing broke.

Bookkeeping

The previous master branch version before the changes in this PR is tagged as v1.5_pre-python3, and copied to the python2 branch.

smonig commented 1 year ago

Hi @IzaakWN , thanks a lot! To me looks good (just never saw this dot when importing classes, like from .TreeProducer import ..., but from google it looks like it should be right). Maybe it would be worth updating the general README as well. In particular:

For now, that's all; I did not try with correctionlib yet.

IzaakWN commented 1 year ago

Hi @smonig,

Thanks a lot for the quick feedback!

Yes, the period for relative import was inserted by 2to3, but it seems to work in python2, so I decided to keep it. Not sure it it's required for python3. It's probably preferable to use TauFW.PicoProducer.analysis... instead.

Adding your suggested instructions to the README is a good idea! It can be added to this PR. I will probably keep the old instructions as well.

IzaakWN commented 1 year ago

After validating mostly the code in TauFW/PicoProducer between a python2 and a python3 setup, I found numerous other issues in the common framework code.

The aim was to keep compatibility with python 2.7 as far as possible to not completely break old setups, and avoid code divergences. However, python 2 has already reached its end of life in 2020, so anyone using nanoAOD v10 samples (produced with CMSSW 12) should definitely move to python 3 as soon as possible. As mentioned above, a python2 branch and v1.5_pre-python3 tag is available before the changes in this PR, although there is no plan to maintain it. To run pico.py in CMSSW 11 and older, one should now use pico2.py instead because of issues with the she-bang (see below).

To summarize the main changes for python 2 and 3 compatibility:

Other unrelated changes I snuck in:

Documentation:

Validation:

Known issues:

I will merge by tomorrow if no further feedback by @cardinia or @smonig.