code-cleaning in RecoMET/METProducers/python #4259
Conversation
|
A new Pull Request was created by @TaiSakuma (Tai Sakuma) for CMSSW_7_2_X. code-cleaning in RecoMET/METProducers/python It involves the following packages: CommonTools/ParticleFlow @nclopezo, @monttj, @cmsbuild, @StoyanStoynev, @slava77, @vadler, @Degano can you please review it and eventually sign? Thanks. |
| @@ -4,7 +4,7 @@ | |||
| from RecoMET.METProducers.PFMET_cfi import * | |||
|
|
|||
| # Should the name be changed se it is similar to pfMet from reco ?? | |||
| pfMET = pfMet.clone(alias="pfMET") | |||
| pfMET = pfMet.clone() | |||
There was a problem hiding this comment.
(this refers to all)
Does it make sense to remove aliases for common names completely?
Some analysis scripts in fwlite rely on these.
What's the strategy now? Let people set the aliases manually?
pfMET.phi() is much simpler to use in fwlite tree draw than the full branch name
|
@slava77, I didn't correctly understand what aliases do. (I will put a link to twiki here so I can find it later https://twiki.cern.ch/twiki/bin/view/CMSPublic/WorkBookFWLite) I will keep the aliases. I changes the aliases of CaloMETs. I thought about to making the aliases of CaloMETs all capitals as in the aliases for several other METs. But it will become harder to recognize, e.g., CALOMETBEFO. So I made them the same as the module names. |
|
Looking into it |
|
@TaiSakuma could you change the first letter of the aliases to capital (letter) to avoid introducing yet another convention. Note that there are several cases like yours in RecoMET/METProducers/python - either all letters are capital or the first one is capital. You may also consider putting comments if you like (see genMetTrue_cfi.py) - useful in a longer run. |
|
I prefer lower case, actually. |
|
@StoyanStoynev, @slava77, if the aliases are the same as the names of the modules, I think that would be simple and convenient. I will make the aliases and the names of all module in RecoMET/METProducers/python the same. is it ok? |
|
The only reason I don't like exact matching is that the uniqueness of the string pattern is broken |
|
The story with capitalization really goes to the coding style/rules that we have: types can start from a capital letter, instances (collection names included) start from lower case. |
|
@TaiSakuma are you making more changes? Other than that all is fine, no diffs expected and none observed (Jenkins; local tests on 21c65e0 with CMSSW_7_2_X_2014-06-17-1400). |
|
@StoyanStoynev, I think that I am done for this PR. I will change aliases of other METs to uniform the convention of the capitalization as a separate PR later. |
|
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). |
code-cleaning in RecoMET/METProducers/python
This PR deletes old definitions of CaloMETs, which were replaced with new definitions at #3942.
This PR also deletes aliases, which became optional at #4006, from several MET definitions.