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

python modernize 2to3 tool fix_filter #23454

Merged

Conversation

davidlange6
Copy link
Contributor

apply fixes.fix_filter tool from python futurize tool

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2018

A new Pull Request was created by @davidlange6 (David Lange) for master.

It involves the following packages:

Alignment/MillePedeAlignmentAlgorithm
CalibTracker/SiStripDCS
CondCore/Utilities
CondTools/BTau
Configuration/DataProcessing
Configuration/PyReleaseValidation
DQM/SiPixelPhase1Common
DQMServices/FileIO
FWCore/GuiBrowsers
HLTriggerOffline/Btag
IOMC/RandomEngine
L1Trigger/L1TMuonBarrel
PhysicsTools/Heppy
PhysicsTools/HeppyCore
PhysicsTools/RooStatsCms
RecoLuminosity/LumiDB
SimMuon/Configuration
Utilities/RelMon
Validation/Configuration
Validation/Performance
Validation/RecoTau
Validation/RecoTrack

@kpedro88, @fabozzi, @nsmith-, @rekovic, @thomreis, @vanbesien, @arizzi, @perrotta, @civanch, @monttj, @cmsbuild, @GurpreetSinghChahal, @davidlange6, @smuzaffar, @Dr15Jones, @mdhildreth, @jfernan2, @cerminar, @slava77, @ggovi, @fabiocos, @prebello, @vazzolini, @kmaeshima, @arunhep, @dmitrijus, @franzoni, @gpetruc, @lpernie can you please review it and eventually sign? Thanks.
@echabert, @felicepantaleo, @abbiendi, @Martin-Grunewald, @fioriNTU, @thomreis, @tlampen, @ahinzmann, @threus, @mmusich, @hdelanno, @calderona, @makortel, @smoortga, @acaudron, @jhgoh, @HuguesBrun, @drkovalskyi, @dildick, @trocino, @barvic, @GiacomoSguazzoni, @rovere, @VinInn, @ferencek, @gkasieczka, @tocheng, @jandrea, @mschrode, @idebruyn, @ebrondol, @mtosi, @dgulhan, @swertz, @battibass, @gbenelli, @wddgit, @JyothsnaKomaragiri, @mverzett, @wmtford, @cbernet, @imarches, @pvmulder, @folguera this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@@ -289,15 +289,15 @@ def sinkPorts(self):
return [port for port in self._ports if port.portType() == "sink"]
def isSink(port):
return port.portType() == 'sink'
return filter(isSink, self._ports)
return list(filter(isSink, self._ports))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we understand this change? Does filter return a generator in python 3 while the old filter returned a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so now I understand you consider this a problem. not sure what alternatives there are. I'll have a look

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems an easy replacement is

return [item for item in self._ports if isSink(item)]

I'd propose to handle the 13 changes (mostly in obsolete code and all in non-performance critical code) for a followup pr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur.

@davidlange6
Copy link
Contributor Author

davidlange6 commented Jun 4, 2018 via email

@davidlange6
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28433/console Started: 2018/06/04 16:49

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2018

@arunhep
Copy link
Contributor

arunhep commented Jun 5, 2018

+1

@kpedro88
Copy link
Contributor

kpedro88 commented Jun 5, 2018

+1

@perrotta
Copy link
Contributor

perrotta commented Jun 9, 2018

+1

  • Technical
  • Jenkins tests pass

@prebello
Copy link
Contributor

+1

@thomreis
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

+operations

the expanded output of the Configuration/DataProcessing unit test is unchanged

@ggovi
Copy link
Contributor

ggovi commented Jun 12, 2018

+1

@Dr15Jones
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment