Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make python3 compatible #36

Merged
merged 55 commits into from
Jun 14, 2023
Merged

Make python3 compatible #36

merged 55 commits into from
Jun 14, 2023

Conversation

IzaakWN
Copy link
Collaborator

@IzaakWN IzaakWN commented May 28, 2023

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 #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

  • converting print statements to print functions: printprint();
  • iteritems()items();
  • xrangerange.

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

  • CMSSW_10_3_3
  • CMSSW_12_4_8

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.

@IzaakWN IzaakWN changed the title Make python3 compatible Make python3 compatible May 28, 2023
@smonig
Copy link
Collaborator

smonig commented May 30, 2023

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:

  • should login to lxplus8 (not lxplus)
  • then setup:
    export CMSSW=CMSSW_12_4_8
    export SCRAM_ARCH=el8_amd64_gcc10
    ...
  • HiggsAnalysis/CombinedLimit --> git checkout v9.1.0 (I think; in the doc, they say it should work, but they explicitely garantie only for CMSSW_11_xxx)
  • For CombineHarvester, need to use now: git checkout v2.0.0 (in the doc, they only mention CMSSW_11, but I guess it should work like this as well...)

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

@IzaakWN
Copy link
Collaborator Author

IzaakWN commented May 30, 2023

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
Copy link
Collaborator Author

IzaakWN commented Jun 12, 2023

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:

  • print statement → print() function.
  • from __future__ import print_function where needed for python2 compatibility.
  • Convert print >> sys.stderr, message to print(message,file=sys.stderr).
  • iteritems()items().
  • xrange()range().
  • keys()list(keys()) to ensure list instead of iterator where needed.
  • raw_inputinput (see bce39ac).
  • Added imports from past.builtins for basestring, unicode, long to keep python2 compatibility where possible.
  • Convert use of exec to eval, see b86ecc3.
  • BTagCalibration::BTagCalibration changed number of arguments in CMSSW 12, see a570634. Setting validate=False should speed up the initiation of the calibrator, see Validate TF1 if requested in BTagEntry cms-sw/cmssw#35856
  • None is not treated as 0 in inequalities of if-statements anymore.

Other unrelated changes I snuck in:

  • Absolute import TreeProducerTauFW.PicoProducer.analysis.TreeProducer.
  • Fixed typo fillCommonCorrBrachesfillCommonCorrBranches everywhere.
  • Split the HTCondor batch system modules for lxplus, KIT, NAF, DESY (see bce39ac).
  • Rename getsedirguess_sedir, gettmpdirsguess_tmpdirs.
  • Implement specialized batch system classes HTCondor_KIT, HTCondor_NAF, etc., and help function guess_batch, see bce39ac.
  • Removed hardcoded workaround for buggy genweight in Summer19UL W+jet samples, see 89bd349.

Documentation:

Validation:

  • scram b -j8 in both CMSSW_10_3_3 and CMSSW_12_4_8 setups to test both python versions.
  • Running some basic pico.py commands in both setups.

Known issues:

  • For CMSSW 11, ROOT works in both python 2.7 and 3, but starting from CMSSW 12, ROOT will not work anymore for python2.
  • Any executable python script with a shebang like #! /usr/bin/env python will still be run with python2 in CMSSW 12, while for CMSSW 13, the python command will be completely defunct. From now on, people should explicitly use python3 in the shebangs, or run the script via a python command. At the moment, some useful scripts with this shebang like haddnano.py from nanoAOD-tools must be run as
python3 $CMSSW_BASE/src/PhysicsTools/NanoAODTools/scripts/haddnano.py

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

@IzaakWN IzaakWN merged commit 950ea86 into cms-tau-pog:master Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants